[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In general I think it would be cool to think all of the problems in the same 
time as these really depends on the `CoreEngine` and how we operates with 
`ExplodedNodes`.

In D58367#1405185 , @NoQ wrote:

> In particular, the implementation you propose does a lot more than adding a 
> note: it also adds, well, a transition.


I think a possible workaround to handle special nodes is adding them in 
`CoreEngine` as successors (instead of predecessors) so when we are traversing 
backwards it immediately known the next (pred) "not-special" node will be its 
parent. This is the same logic in `ConditionBRVisitor` where we only work with 
pair of nodes to see if we would report something.

> The issue here is that the checker doesn't always have an ability to mutate 
> the graph by adding nodes. Now it becomes more of a problem because the 
> checker may want to emit notes more often than it wants to generate nodes. 
> This problem can be easily mitigated by passing a different sort of context 
> (not `CheckerContext` but something else) into these callbacks that will 
> store lambdas in a different manner.
>  I think this is not a big flexibility issue on its own because it should be 
> possible to make lambdas from multiple sources cooperate with each other by 
> sharing a certain amount of arbitrary state information within the BugReport 
> object.

I think it ties with the problem above, so having some special `ExplodedNode` 
could fix both of them because may you would introduce some 
`ExplodedNodeContext` to store more information.

> In D58367#1404722 , @Charusso wrote:
> 
>>   const NoteTag *setNoteTag(StringRef Message) {
>> return Eng.getNoteTags().setNoteTag(Message);
>>   }
>>   
> 
> 
> Mmm, i don't think i understand this part. Like, `getNoteTag()` is not really 
> a getter. It's a factory method on an allocator that causes it to produce an 
> object. What is the semantics of a setter that would correspond to it?

All the problems what you mentioned is not really a job of a factory, so I just 
emphasized we only get the true benefit of the callback in the get part. I 
think leave it as it is now makes sense but later on there would be a lot more 
function to hook crazy lambdas to *set* information.

Please note that it is a very high-level feedback, as I just checked the 
`CoreEngine` and saw why we are splitting states.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58367



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

[PATCH] D53701: [Analyzer] Instead of recording comparisons in interator checkers do an eager state split

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

baloghadamsoftware updated this revision to Diff 187732.
baloghadamsoftware added a comment.

`processComparison()` refactored.


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

https://reviews.llvm.org/D53701

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -133,8 +133,6 @@
   }
 };
 
-typedef llvm::PointerUnion RegionOrSymbol;
-
 // Structure to record the symbolic begin and end position of a container
 struct ContainerData {
 private:
@@ -172,41 +170,21 @@
   }
 };
 
-// Structure fo recording iterator comparisons. We needed to retrieve the
-// original comparison expression in assumptions.
-struct IteratorComparison {
-private:
-  RegionOrSymbol Left, Right;
-  bool Equality;
-
-public:
-  IteratorComparison(RegionOrSymbol L, RegionOrSymbol R, bool Eq)
-  : Left(L), Right(R), Equality(Eq) {}
-
-  RegionOrSymbol getLeft() const { return Left; }
-  RegionOrSymbol getRight() const { return Right; }
-  bool isEquality() const { return Equality; }
-  bool operator==(const IteratorComparison &X) const {
-return Left == X.Left && Right == X.Right && Equality == X.Equality;
-  }
-  bool operator!=(const IteratorComparison &X) const {
-return Left != X.Left || Right != X.Right || Equality != X.Equality;
-  }
-  void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(Equality); }
-};
-
 class IteratorChecker
 : public Checker, check::Bind,
- check::LiveSymbols, check::DeadSymbols,
- eval::Assume> {
+ check::LiveSymbols, check::DeadSymbols> {
 
   std::unique_ptr OutOfRangeBugType;
   std::unique_ptr MismatchedBugType;
   std::unique_ptr InvalidatedBugType;
 
-  void handleComparison(CheckerContext &C, const SVal &RetVal, const SVal &LVal,
-const SVal &RVal, OverloadedOperatorKind Op) const;
+  void handleComparison(CheckerContext &C, const Expr *CE, const SVal &RetVal,
+const SVal &LVal, const SVal &RVal,
+OverloadedOperatorKind Op) const;
+  void processComparison(CheckerContext &C, ProgramStateRef State,
+ SymbolRef Sym1, SymbolRef Sym2, const SVal &RetVal,
+ OverloadedOperatorKind Op) const;
   void verifyAccess(CheckerContext &C, const SVal &Val) const;
   void verifyDereference(CheckerContext &C, const SVal &Val) const;
   void handleIncrement(CheckerContext &C, const SVal &RetVal, const SVal &Iter,
@@ -281,8 +259,6 @@
  CheckerContext &C) const;
   void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
-  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
- bool Assumption) const;
 };
 } // namespace
 
@@ -292,9 +268,6 @@
 
 REGISTER_MAP_WITH_PROGRAMSTATE(ContainerMap, const MemRegion *, ContainerData)
 
-REGISTER_MAP_WITH_PROGRAMSTATE(IteratorComparisonMap, const SymExpr *,
-   IteratorComparison)
-
 namespace {
 
 bool isIteratorType(const QualType &Type);
@@ -324,16 +297,6 @@
 bool hasSubscriptOperator(ProgramStateRef State, const MemRegion *Reg);
 bool frontModifiable(ProgramStateRef State, const MemRegion *Reg);
 bool backModifiable(ProgramStateRef State, const MemRegion *Reg);
-BinaryOperator::Opcode getOpcode(const SymExpr *SE);
-const RegionOrSymbol getRegionOrSymbol(const SVal &Val);
-const ProgramStateRef processComparison(ProgramStateRef State,
-RegionOrSymbol LVal,
-RegionOrSymbol RVal, bool Equal);
-const ProgramStateRef saveComparison(ProgramStateRef State,
- const SymExpr *Condition, const SVal &LVal,
- const SVal &RVal, bool Eq);
-const IteratorComparison *loadComparison(ProgramStateRef State,
- const SymExpr *Condition);
 SymbolRef getContainerBegin(ProgramStateRef State, const MemRegion *Cont);
 SymbolRef getContainerEnd(ProgramStateRef State, const MemRegion *Cont);
 ProgramStateRef createContainerBegin(ProgramStateRef State,
@@ -343,21 +306,9 @@
const SymbolRef Sym);
 const IteratorPosition *getIteratorPosition(ProgramStateRef State,
 const SVal &Val);
-const IteratorPosition *getIteratorPosition(ProgramStateRef State,
-RegionOrSymbol RegOrSym);
 ProgramStateRef setIteratorPosition(ProgramStateRef State, const SVal &Val,
 const IteratorPosition &Pos);
-ProgramStateRef setIteratorPosition(ProgramStateRef State,
-   

[PATCH] D57896: Variable names rule

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D57896#1402280 , @zturner wrote:

> Since someone already accepted this, I suppose I should mark require changes 
> to formalize my dissent


As it was Chris @lattner who accepted it, is your request for changes just 
based on the fact that it doesn't fit LLDB style?

I was trying to find where the LLDB coding style was documented but could only 
find this 
https://llvm.org/svn/llvm-project/lldb/branches/release_36/www/lldb-coding-conventions.html,
 seemingly this file has been move/removed around release 3.9.

But reading that link its seems unlikely to find a concencous between either 
naming conventions or formatting style between LLDB and the rest of LLVM, 
unless of course the solution would be to adopt LLDB style completely (for 
which I'd suspect there would be counter objections)

If that isn't a reality is blocking the rest of the LLVM community from 
relieving some of their eye strain an acceptable solution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[PATCH] D58492: [clangd] Add thread priority lowering for MacOS as well

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58492

Files:
  clangd/Threading.cpp


Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -121,6 +121,12 @@
   Priority == ThreadPriority::Low && !AvoidThreadStarvation ? SCHED_IDLE
 : SCHED_OTHER,
   &priority);
+#elif defined(__APPLE__)
+  // 
https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/getpriority.2.html
+  setpriority(PRIO_DARWIN_THREAD, 0,
+  Priority == ThreadPriority::Low && !AvoidThreadStarvation
+  ? PRIO_DARWIN_BG
+  : 0);
 #endif
 }
 


Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -121,6 +121,12 @@
   Priority == ThreadPriority::Low && !AvoidThreadStarvation ? SCHED_IDLE
 : SCHED_OTHER,
   &priority);
+#elif defined(__APPLE__)
+  // https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/getpriority.2.html
+  setpriority(PRIO_DARWIN_THREAD, 0,
+  Priority == ThreadPriority::Low && !AvoidThreadStarvation
+  ? PRIO_DARWIN_BG
+  : 0);
 #endif
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r354558 - [clangd] Handle another incomplete-type diagnostic case in IncludeFixer.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Thu Feb 21 01:33:49 2019
New Revision: 354558

URL: http://llvm.org/viewvc/llvm-project?rev=354558&view=rev
Log:
[clangd] Handle another incomplete-type diagnostic case in IncludeFixer.

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

Modified: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/IncludeFixer.cpp?rev=354558&r1=354557&r2=354558&view=diff
==
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp Thu Feb 21 01:33:49 2019
@@ -67,6 +67,7 @@ std::vector IncludeFixer::fix(Diagn
   case diag::err_incomplete_type:
   case diag::err_incomplete_member_access:
   case diag::err_incomplete_base_class:
+  case diag::err_incomplete_nested_name_spec:
 // Incomplete type diagnostics should have a QualType argument for the
 // incomplete type.
 for (unsigned Idx = 0; Idx < Info.getNumArgs(); ++Idx) {

Modified: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp?rev=354558&r1=354557&r2=354558&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp Thu Feb 21 
01:33:49 2019
@@ -311,6 +311,7 @@ TEST(IncludeFixerTest, IncompleteType) {
   Annotations Test(R"cpp(
 $insert[[]]namespace ns {
   class X;
+  $nested[[X::]]Nested n;
 }
 class Y : $base[[public ns::X]] {};
 int main() {
@@ -326,6 +327,10 @@ int main() {
   EXPECT_THAT(
   TU.build().getDiagnostics(),
   UnorderedElementsAre(
+  AllOf(Diag(Test.range("nested"),
+ "incomplete type 'ns::X' named in nested name specifier"),
+WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+"Add include \"x.h\" for symbol ns::X"))),
   AllOf(Diag(Test.range("base"), "base class has incomplete type"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),


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

[PATCH] D58293: [clang][Index] Enable indexing of Template Type Parameters behind a flag

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase


Repository:
  rC Clang

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

https://reviews.llvm.org/D58293

Files:
  include/clang/Index/IndexingAction.h
  lib/Index/IndexDecl.cpp
  lib/Index/IndexSymbol.cpp
  lib/Index/IndexTypeSourceInfo.cpp
  lib/Index/IndexingContext.cpp
  lib/Index/IndexingContext.h
  unittests/Index/IndexTests.cpp

Index: unittests/Index/IndexTests.cpp
===
--- unittests/Index/IndexTests.cpp
+++ unittests/Index/IndexTests.cpp
@@ -216,6 +216,30 @@
  DeclAt(Position(5, 12);
 }
 
+TEST(IndexTest, IndexTypeParmDecls) {
+  std::string Code = R"cpp(
+template  class C, typename NoRef>
+struct Foo {
+  T t = I;
+  C x;
+};
+  )cpp";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, AllOf(Not(Contains(QName("Foo::T"))),
+Not(Contains(QName("Foo::I"))),
+Not(Contains(QName("Foo::C"))),
+Not(Contains(QName("Foo::NoRef");
+
+  Opts.IndexTemplateParameters = true;
+  Index->Symbols.clear();
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols,
+  AllOf(Contains(QName("Foo::T")), Contains(QName("Foo::I")),
+Contains(QName("Foo::C")), Contains(QName("Foo::NoRef";
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: lib/Index/IndexingContext.h
===
--- lib/Index/IndexingContext.h
+++ lib/Index/IndexingContext.h
@@ -63,6 +63,8 @@
 
   bool shouldIndexParametersInDeclarations() const;
 
+  bool shouldIndexTemplateParameters() const;
+
   static bool isTemplateImplicitInstantiation(const Decl *D);
 
   bool handleDecl(const Decl *D, SymbolRoleSet Roles = SymbolRoleSet(),
Index: lib/Index/IndexingContext.cpp
===
--- lib/Index/IndexingContext.cpp
+++ lib/Index/IndexingContext.cpp
@@ -44,6 +44,10 @@
   return IndexOpts.IndexParametersInDeclarations;
 }
 
+bool IndexingContext::shouldIndexTemplateParameters() const {
+  return IndexOpts.IndexTemplateParameters;
+}
+
 bool IndexingContext::handleDecl(const Decl *D,
  SymbolRoleSet Roles,
  ArrayRef Relations) {
@@ -76,8 +80,11 @@
   if (!shouldIndexFunctionLocalSymbols() && isFunctionLocalSymbol(D))
 return true;
 
-  if (isa(D) || isa(D))
+  if (!shouldIndexTemplateParameters() &&
+  (isa(D) || isa(D) ||
+   isa(D))) {
 return true;
+  }
 
   return handleDeclOccurrence(D, Loc, /*IsRef=*/true, Parent, Roles, Relations,
   RefE, RefD, DC);
Index: lib/Index/IndexTypeSourceInfo.cpp
===
--- lib/Index/IndexTypeSourceInfo.cpp
+++ lib/Index/IndexTypeSourceInfo.cpp
@@ -45,6 +45,13 @@
   return false;\
   } while (0)
 
+  bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TTPL) {
+SourceLocation Loc = TTPL.getNameLoc();
+TemplateTypeParmDecl *TTPD = TTPL.getDecl();
+return IndexCtx.handleReference(TTPD, Loc, Parent, ParentDC,
+SymbolRoleSet());
+  }
+
   bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
 SourceLocation Loc = TL.getNameLoc();
 TypedefNameDecl *ND = TL.getTypedefNameDecl();
Index: lib/Index/IndexSymbol.cpp
===
--- lib/Index/IndexSymbol.cpp
+++ lib/Index/IndexSymbol.cpp
@@ -55,9 +55,6 @@
   if (isa(D))
 return true;
 
-  if (isa(D))
-return true;
-
   if (isa(D))
 return true;
 
Index: lib/Index/IndexDecl.cpp
===
--- lib/Index/IndexDecl.cpp
+++ lib/Index/IndexDecl.cpp
@@ -672,6 +672,8 @@
 shouldIndexTemplateParameterDefaultValue(Parent)) {
   const TemplateParameterList *Params = D->getTemplateParameters();
   for (const NamedDecl *TP : *Params) {
+if (IndexCtx.shouldIndexTemplateParameters())
+  IndexCtx.handleDecl(TP);
 if (const auto *TTP = dyn_cast(TP)) {
   if (TTP->hasDefaultArgument())
 IndexCtx.indexTypeSourceInfo(TTP->getDefaultArgumentInfo(), Parent);
Index: include/clang/Index/IndexingAction.h
===
--- include/clang/Index/IndexingAction.h
+++ include/clang/Index/IndexingAction.h
@@ -46,6 +46,7 @@
   bool IndexMacrosInPreprocessor = false;
   // Has no effect if IndexFunctionLocals are false.
   bool IndexParametersInDeclarations = false;
+  bool Index

r354560 - [clang][Index] Enable indexing of Template Type Parameters behind a flag

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Feb 21 01:52:33 2019
New Revision: 354560

URL: http://llvm.org/viewvc/llvm-project?rev=354560&view=rev
Log:
[clang][Index] Enable indexing of Template Type Parameters behind a flag

Summary:
clangd uses indexing api to provide references and it was not possible
to perform symbol information for template parameters. This patch enables
visiting of TemplateTypeParmTypeLocs.

Reviewers: ilya-biryukov, akyrtzi

Subscribers: javed.absar, kristof.beyls, ioeric, arphaman, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Index/IndexingAction.h
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/lib/Index/IndexSymbol.cpp
cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
cfe/trunk/lib/Index/IndexingContext.cpp
cfe/trunk/lib/Index/IndexingContext.h
cfe/trunk/unittests/Index/IndexTests.cpp

Modified: cfe/trunk/include/clang/Index/IndexingAction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/IndexingAction.h?rev=354560&r1=354559&r2=354560&view=diff
==
--- cfe/trunk/include/clang/Index/IndexingAction.h (original)
+++ cfe/trunk/include/clang/Index/IndexingAction.h Thu Feb 21 01:52:33 2019
@@ -46,6 +46,7 @@ struct IndexingOptions {
   bool IndexMacrosInPreprocessor = false;
   // Has no effect if IndexFunctionLocals are false.
   bool IndexParametersInDeclarations = false;
+  bool IndexTemplateParameters = false;
 };
 
 /// Creates a frontend action that indexes all symbols (macros and AST decls).

Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=354560&r1=354559&r2=354560&view=diff
==
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Thu Feb 21 01:52:33 2019
@@ -672,6 +672,8 @@ public:
 shouldIndexTemplateParameterDefaultValue(Parent)) {
   const TemplateParameterList *Params = D->getTemplateParameters();
   for (const NamedDecl *TP : *Params) {
+if (IndexCtx.shouldIndexTemplateParameters())
+  IndexCtx.handleDecl(TP);
 if (const auto *TTP = dyn_cast(TP)) {
   if (TTP->hasDefaultArgument())
 IndexCtx.indexTypeSourceInfo(TTP->getDefaultArgumentInfo(), 
Parent);

Modified: cfe/trunk/lib/Index/IndexSymbol.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexSymbol.cpp?rev=354560&r1=354559&r2=354560&view=diff
==
--- cfe/trunk/lib/Index/IndexSymbol.cpp (original)
+++ cfe/trunk/lib/Index/IndexSymbol.cpp Thu Feb 21 01:52:33 2019
@@ -55,9 +55,6 @@ bool index::isFunctionLocalSymbol(const
   if (isa(D))
 return true;
 
-  if (isa(D))
-return true;
-
   if (isa(D))
 return true;
 

Modified: cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp?rev=354560&r1=354559&r2=354560&view=diff
==
--- cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp (original)
+++ cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp Thu Feb 21 01:52:33 2019
@@ -45,6 +45,13 @@ public:
   return false;
\
   } while (0)
 
+  bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TTPL) {
+SourceLocation Loc = TTPL.getNameLoc();
+TemplateTypeParmDecl *TTPD = TTPL.getDecl();
+return IndexCtx.handleReference(TTPD, Loc, Parent, ParentDC,
+SymbolRoleSet());
+  }
+
   bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
 SourceLocation Loc = TL.getNameLoc();
 TypedefNameDecl *ND = TL.getTypedefNameDecl();

Modified: cfe/trunk/lib/Index/IndexingContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingContext.cpp?rev=354560&r1=354559&r2=354560&view=diff
==
--- cfe/trunk/lib/Index/IndexingContext.cpp (original)
+++ cfe/trunk/lib/Index/IndexingContext.cpp Thu Feb 21 01:52:33 2019
@@ -44,6 +44,10 @@ bool IndexingContext::shouldIndexParamet
   return IndexOpts.IndexParametersInDeclarations;
 }
 
+bool IndexingContext::shouldIndexTemplateParameters() const {
+  return IndexOpts.IndexTemplateParameters;
+}
+
 bool IndexingContext::handleDecl(const Decl *D,
  SymbolRoleSet Roles,
  ArrayRef Relations) {
@@ -76,8 +80,11 @@ bool IndexingContext::handleReference(co
   if (!shouldIndexFunctionLocalSymbols() && isFunctionLocalSymbol(D))
 return true;
 
-  if (isa(D) || isa(D))
+  if (!shouldIndexTemplateParameters() &&
+  (isa(D) || isa(D) ||
+   isa(D))) {
 return true;
+  }
 
   return handleDeclOc

[PATCH] D58293: [clang][Index] Enable indexing of Template Type Parameters behind a flag

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC354560: [clang][Index] Enable indexing of Template Type 
Parameters behind a flag (authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58293?vs=187738&id=187739#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58293

Files:
  include/clang/Index/IndexingAction.h
  lib/Index/IndexDecl.cpp
  lib/Index/IndexSymbol.cpp
  lib/Index/IndexTypeSourceInfo.cpp
  lib/Index/IndexingContext.cpp
  lib/Index/IndexingContext.h
  unittests/Index/IndexTests.cpp

Index: lib/Index/IndexTypeSourceInfo.cpp
===
--- lib/Index/IndexTypeSourceInfo.cpp
+++ lib/Index/IndexTypeSourceInfo.cpp
@@ -45,6 +45,13 @@
   return false;\
   } while (0)
 
+  bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TTPL) {
+SourceLocation Loc = TTPL.getNameLoc();
+TemplateTypeParmDecl *TTPD = TTPL.getDecl();
+return IndexCtx.handleReference(TTPD, Loc, Parent, ParentDC,
+SymbolRoleSet());
+  }
+
   bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
 SourceLocation Loc = TL.getNameLoc();
 TypedefNameDecl *ND = TL.getTypedefNameDecl();
Index: lib/Index/IndexingContext.h
===
--- lib/Index/IndexingContext.h
+++ lib/Index/IndexingContext.h
@@ -63,6 +63,8 @@
 
   bool shouldIndexParametersInDeclarations() const;
 
+  bool shouldIndexTemplateParameters() const;
+
   static bool isTemplateImplicitInstantiation(const Decl *D);
 
   bool handleDecl(const Decl *D, SymbolRoleSet Roles = SymbolRoleSet(),
Index: lib/Index/IndexDecl.cpp
===
--- lib/Index/IndexDecl.cpp
+++ lib/Index/IndexDecl.cpp
@@ -672,6 +672,8 @@
 shouldIndexTemplateParameterDefaultValue(Parent)) {
   const TemplateParameterList *Params = D->getTemplateParameters();
   for (const NamedDecl *TP : *Params) {
+if (IndexCtx.shouldIndexTemplateParameters())
+  IndexCtx.handleDecl(TP);
 if (const auto *TTP = dyn_cast(TP)) {
   if (TTP->hasDefaultArgument())
 IndexCtx.indexTypeSourceInfo(TTP->getDefaultArgumentInfo(), Parent);
Index: lib/Index/IndexSymbol.cpp
===
--- lib/Index/IndexSymbol.cpp
+++ lib/Index/IndexSymbol.cpp
@@ -55,9 +55,6 @@
   if (isa(D))
 return true;
 
-  if (isa(D))
-return true;
-
   if (isa(D))
 return true;
 
Index: lib/Index/IndexingContext.cpp
===
--- lib/Index/IndexingContext.cpp
+++ lib/Index/IndexingContext.cpp
@@ -44,6 +44,10 @@
   return IndexOpts.IndexParametersInDeclarations;
 }
 
+bool IndexingContext::shouldIndexTemplateParameters() const {
+  return IndexOpts.IndexTemplateParameters;
+}
+
 bool IndexingContext::handleDecl(const Decl *D,
  SymbolRoleSet Roles,
  ArrayRef Relations) {
@@ -76,8 +80,11 @@
   if (!shouldIndexFunctionLocalSymbols() && isFunctionLocalSymbol(D))
 return true;
 
-  if (isa(D) || isa(D))
+  if (!shouldIndexTemplateParameters() &&
+  (isa(D) || isa(D) ||
+   isa(D))) {
 return true;
+  }
 
   return handleDeclOccurrence(D, Loc, /*IsRef=*/true, Parent, Roles, Relations,
   RefE, RefD, DC);
Index: unittests/Index/IndexTests.cpp
===
--- unittests/Index/IndexTests.cpp
+++ unittests/Index/IndexTests.cpp
@@ -216,6 +216,30 @@
  DeclAt(Position(5, 12);
 }
 
+TEST(IndexTest, IndexTypeParmDecls) {
+  std::string Code = R"cpp(
+template  class C, typename NoRef>
+struct Foo {
+  T t = I;
+  C x;
+};
+  )cpp";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, AllOf(Not(Contains(QName("Foo::T"))),
+Not(Contains(QName("Foo::I"))),
+Not(Contains(QName("Foo::C"))),
+Not(Contains(QName("Foo::NoRef");
+
+  Opts.IndexTemplateParameters = true;
+  Index->Symbols.clear();
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols,
+  AllOf(Contains(QName("Foo::T")), Contains(QName("Foo::I")),
+Contains(QName("Foo::C")), Contains(QName("Foo::NoRef";
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: include/clang/Index/IndexingAction.h
===
--- include/clang/Index/IndexingAction.h
++

[clang-tools-extra] r354561 - [clangd] Enable indexing of template type parameters

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Feb 21 01:55:00 2019
New Revision: 354561

URL: http://llvm.org/viewvc/llvm-project?rev=354561&view=rev
Log:
[clangd] Enable indexing of template type parameters

Summary: Fixes https://bugs.llvm.org/show_bug.cgi?id=36285

Reviewers: ilya-biryukov

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolInfoTests.cpp
clang-tools-extra/trunk/unittests/clangd/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=354561&r1=354560&r2=354561&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Thu Feb 21 01:55:00 2019
@@ -39,7 +39,8 @@ const Decl *getDefinition(const Decl *D)
   if (const auto *FD = dyn_cast(D))
 return FD->getDefinition();
   // Only a single declaration is allowed.
-  if (isa(D)) // except cases above
+  if (isa(D) || isa(D) ||
+  isa(D)) // except cases above
 return D;
   // Multiple definitions are allowed.
   return nullptr; // except cases above
@@ -243,6 +244,7 @@ IdentifiedSymbol getSymbolAtPosition(Par
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParameters = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
@@ -441,6 +443,7 @@ findRefs(const std::vector
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParameters = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), RefFinder, IndexOpts);
   return std::move(RefFinder).take();

Modified: clang-tools-extra/trunk/unittests/clangd/SymbolInfoTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolInfoTests.cpp?rev=354561&r1=354560&r2=354561&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SymbolInfoTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolInfoTests.cpp Thu Feb 21 
01:55:00 2019
@@ -213,14 +213,14 @@ TEST(SymbolInfoTests, All) {
 T^T t;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("TT", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Template parameter reference - type param
   template struct bar {
 int a = N^N;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("NN", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Class member reference - objec
   struct foo {

Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=354561&r1=354560&r2=354561&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Thu Feb 21 01:55:00 
2019
@@ -285,11 +285,15 @@ TEST(LocateSymbol, All) {
 }
   )cpp",
 
-  /* FIXME: clangIndex doesn't handle template type parameters
   R"cpp(// Template type parameter
-template <[[typename T]]>
+template 
 void foo() { ^T t; }
-  )cpp", */
+  )cpp",
+
+  R"cpp(// Template template type parameter
+template  class [[T]]>
+void foo() { ^T t; }
+  )cpp",
 
   R"cpp(// Namespace
 namespace $decl[[ns]] {


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

[PATCH] D58294: [clangd] Enable indexing of template type parameters

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE354561: [clangd] Enable indexing of template type 
parameters (authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58294?vs=187371&id=187741#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58294

Files:
  clangd/XRefs.cpp
  unittests/clangd/SymbolInfoTests.cpp
  unittests/clangd/XRefsTests.cpp


Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -39,7 +39,8 @@
   if (const auto *FD = dyn_cast(D))
 return FD->getDefinition();
   // Only a single declaration is allowed.
-  if (isa(D)) // except cases above
+  if (isa(D) || isa(D) ||
+  isa(D)) // except cases above
 return D;
   // Multiple definitions are allowed.
   return nullptr; // except cases above
@@ -243,6 +244,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParameters = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
@@ -441,6 +443,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParameters = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), RefFinder, IndexOpts);
   return std::move(RefFinder).take();
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -285,11 +285,15 @@
 }
   )cpp",
 
-  /* FIXME: clangIndex doesn't handle template type parameters
   R"cpp(// Template type parameter
-template <[[typename T]]>
+template 
 void foo() { ^T t; }
-  )cpp", */
+  )cpp",
+
+  R"cpp(// Template template type parameter
+template  class [[T]]>
+void foo() { ^T t; }
+  )cpp",
 
   R"cpp(// Namespace
 namespace $decl[[ns]] {
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -213,14 +213,14 @@
 T^T t;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("TT", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Template parameter reference - type param
   template struct bar {
 int a = N^N;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("NN", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Class member reference - objec
   struct foo {


Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -39,7 +39,8 @@
   if (const auto *FD = dyn_cast(D))
 return FD->getDefinition();
   // Only a single declaration is allowed.
-  if (isa(D)) // except cases above
+  if (isa(D) || isa(D) ||
+  isa(D)) // except cases above
 return D;
   // Multiple definitions are allowed.
   return nullptr; // except cases above
@@ -243,6 +244,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParameters = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
@@ -441,6 +443,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParameters = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), RefFinder, IndexOpts);
   return std::move(RefFinder).take();
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -285,11 +285,15 @@
 }
   )cpp",
 
-  /* FIXME: clangIndex doesn't handle template type parameters
   R"cpp(// Template type parameter
-template <[[typename T]]>
+template 
 void foo() { ^T t; }
-  )cpp", */
+  )cpp",
+
+  R"cpp(// Template template type parameter
+template  class [[T]]>
+void foo() { ^T t; }
+  )cpp",
 
   R"cpp(// Namespace
 na

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Could you add some text into the docs/ReleaseNotes.rst to say you are adding a 
new option

take a look at 
https://clang.llvm.org/extra/ReleaseNotes.html#improvements-to-clang-tidy for 
an example


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

https://reviews.llvm.org/D52150



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

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: lib/Format/UnwrappedLineFormatter.cpp:15
 #include 
 
 #define DEBUG_TYPE "format-formatter"

Nice! its the irony of clang-format code is that its not clang formatted!!


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

https://reviews.llvm.org/D52150



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

[PATCH] D58178: [clang-tidy] RawStringLiteralCheck: isRawStringLiteral doesn't check all erroneous cases

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Could I ask if you don't want to pursue this review that you Abandon it.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58178



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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Could you add a line in the docs/ReleaseNotes.rst to say what you are adding


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

https://reviews.llvm.org/D57687



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

[PATCH] D58494: [ASTImporter] Handle redecl chain of FunctionTemplateDecls

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: a_sidorin, shafik.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.

Redecl chains of function templates are not handled well currently. We
want to handle them similarly to functions, i.e. try to keep the
structure of the original AST as much as possible. The aim is to not
squash a prototype with a definition, rather we create both and put them
in a redecl chain.


Repository:
  rC Clang

https://reviews.llvm.org/D58494

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -4163,9 +4163,8 @@
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
 RedeclChain, Variable, ,
 PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition)
-// FIXME Enable this test, once we import function templates chains correctly.
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
-RedeclChain, FunctionTemplate, DISABLED_,
+RedeclChain, FunctionTemplate, ,
 PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
 RedeclChain, ClassTemplate, ,
@@ -4180,9 +4179,8 @@
 RedeclChain, Class, , DefinitionShouldBeImportedAsADefinition)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
 RedeclChain, Variable, , DefinitionShouldBeImportedAsADefinition)
-// FIXME Enable this test, once we import function templates chains correctly.
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
-RedeclChain, FunctionTemplate, DISABLED_,
+RedeclChain, FunctionTemplate, ,
 DefinitionShouldBeImportedAsADefinition)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
 RedeclChain, ClassTemplate, , DefinitionShouldBeImportedAsADefinition)
@@ -4196,9 +4194,7 @@
 ImportPrototypeAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Variable, ,
 ImportPrototypeAfterImportedPrototype)
-// FIXME Enable this test, once we import function templates chains correctly.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate,
-DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate, ,
 ImportPrototypeAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, ,
 ImportPrototypeAfterImportedPrototype)
@@ -4212,9 +4208,7 @@
 ImportDefinitionAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Variable, ,
 ImportDefinitionAfterImportedPrototype)
-// FIXME Enable this test, once we import function templates chains correctly.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate,
-DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate, ,
 ImportDefinitionAfterImportedPrototype)
 // FIXME This does not pass, possible error with ClassTemplate import.
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, DISABLED_,
@@ -4229,9 +4223,7 @@
 ImportPrototypeAfterImportedDefinition)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Variable, ,
 ImportPrototypeAfterImportedDefinition)
-// FIXME Enable this test, once we import function templates chains correctly.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate,
-DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate, ,
 ImportPrototypeAfterImportedDefinition)
 // FIXME This does not pass, possible error with ClassTemplate import.
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, DISABLED_,
@@ -4244,9 +4236,8 @@
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, , ImportPrototypes)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Variable, ,
 ImportPrototypes)
-// FIXME Enable this test, once we import function templates chains correctly.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate,
-DISABLED_, ImportPrototypes)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate, ,
+ImportPrototypes)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, ,
 ImportPrototypes)
 // FIXME This does not pass, possible error with Spec import.
@@ -4259,9 +4250,8 @@
   

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/Format.cpp:649
   LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
+  LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;

What is the difference between what clang-format does now and using SLS_All, 
i.e. if your introducing a new style shouldn't the default be to not change the 
exsiting code?

Without trying this myself I would think this needs to be SLS_None? (am I 
wrong?)


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

https://reviews.llvm.org/D57687



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

[PATCH] D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface

[Pascal Wiesmann via Phabricator via cfe-commits]

wipascal added a comment.

In D10833#1394154 , @dyhe83 wrote:

> In D10833#970906 , @milianw wrote:
>
> > still looks good to me. can someone else please review and commit this?
>
>
> ping


please


Repository:
  rC Clang

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

https://reviews.llvm.org/D10833



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

[PATCH] D58495: [clangd] Only report explicitly typed symbols during code navigation

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Clangd was reporting implicit symbols, like results of implicit cast
expressions during code navigation, which is not desired. For example:

  struct Foo{ Foo(int); };
  void bar(Foo);
  vod foo() {
int x;
bar(^x);
  }

Performing a GoTo on the point specified by ^ would give two results one
pointing to line `int x` and the other for definition of `Foo(int);`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58495

Files:
  clangd/XRefs.cpp
  unittests/clangd/SymbolInfoTests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -406,6 +406,16 @@
 
 double y = va^r;
   )cpp",
+
+  R"cpp(// No implicit constructors
+class X {
+  X(X&& x) = default;
+};
+X [[makeX]]() {}
+void foo() {
+  auto x = m^akeX();
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
@@ -457,16 +467,12 @@
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
   // Ordered assertions are deliberate: we expect a predictable order.
-  EXPECT_THAT(locateSymbolAt(AST, T.point("1")),
-  ElementsAre(Sym("str"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("1")), ElementsAre(Sym("str")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("2")), ElementsAre(Sym("str")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("3")),
-  ElementsAre(Sym("f"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("3")), ElementsAre(Sym("f")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("5")),
-  ElementsAre(Sym("f"), Sym("Foo")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("6")),
-  ElementsAre(Sym("str"), Sym("Foo"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("5")), ElementsAre(Sym("f")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("6")), ElementsAre(Sym("str")));
 }
 
 TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -180,12 +180,8 @@
 func_baz1(f^f);
   }
 )cpp",
-  {
-  CreateExpectedSymbolDetails(
-  "ff", "func_baz2", "c:TestTU.cpp@218@F@func_baz2#@ff"),
-  CreateExpectedSymbolDetails(
-  "bar", "bar::", "c:@S@bar@F@bar#&1$@S@foo#"),
-  }},
+  {CreateExpectedSymbolDetails(
+  "ff", "func_baz2", "c:TestTU.cpp@218@F@func_baz2#@ff")}},
   {
   R"cpp( // Type reference - declaration
   struct foo;
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -123,7 +123,7 @@
   // explicitly in the code.
   // True means the declaration is explicitly referenced at least once; false
   // otherwise.
-  llvm::DenseMap Decls;
+  llvm::DenseSet Decls;
   const SourceLocation &SearchedLocation;
   const ASTContext *
   Preprocessor &PP;
@@ -133,22 +133,14 @@
  ASTContext &AST, Preprocessor &PP)
   : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
 
-  // Get all DeclInfo of the found declarations.
-  // The results are sorted by "IsReferencedExplicitly" and declaration
-  // location.
-  std::vector getFoundDecls() const {
-std::vector Result;
-for (auto It : Decls) {
-  Result.emplace_back();
-  Result.back().D = It.first;
-  Result.back().IsReferencedExplicitly = It.second;
-}
+  // The results are sorted by declaration location.
+  std::vector getFoundDecls() const {
+std::vector Result;
+for (const Decl *D : Decls)
+  Result.push_back(D);
 
-// Sort results. Declarations being referenced explicitly come first.
-llvm::sort(Result, [](const DeclInfo &L, const DeclInfo &R) {
-  if (L.IsReferencedExplicitly != R.IsReferencedExplicitly)
-return L.IsReferencedExplicitly > R.IsReferencedExplicitly;
-  return L.D->getBeginLoc() < R.D->getBeginLoc();
+llvm::sort(Result, [](const Decl *L, const Decl *R) {
+  return L->getBeginLoc() < R->getBeginLoc();
 });
 return Result;
   }
@@ -185,16 +177,17 @@
 return isa(E);
   };
 
-  bool IsExplicit = !IsImplicitExpr(ASTNode.OrigE);
+  if (IsImplicitExpr(ASTNode.OrigE))
+return true;
   // Find and add definition declarations (for GoToDefinition).
   // We don't use parameter `D`, as Parameter `D` i

[PATCH] D58178: [clang-tidy] RawStringLiteralCheck: isRawStringLiteral doesn't check all erroneous cases

[Goran Mitrovic via Phabricator via cfe-commits]

gmit added a comment.

In D58178#1405397 , @MyDeveloperDay 
wrote:

> Could I ask if you don't want to pursue this review that you Abandon it.


Sure, I already wanted that, but I don't see any options that would result in 
closing the issue. Could you, please, point me to it? Tnx!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58178



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

[PATCH] D41720: [clang-tidy] Add a -show-color flag.

[Jan Niklas Hasse via Phabricator via cfe-commits]

jhasse added a comment.

What are you thoughts about using an environment variable for this? Due to the 
number of differently named flags, it's often hard to configure a CI system to 
display color in every tool. (I'm trying to push for a standard for this, see 
https://bixense.com/clicolors/)


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

https://reviews.llvm.org/D41720



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

[PATCH] D58178: [clang-tidy] RawStringLiteralCheck: isRawStringLiteral doesn't check all erroneous cases

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D58178#1405430 , @gmit wrote:

> In D58178#1405397 , @MyDeveloperDay 
> wrote:
>
> > Could I ask if you don't want to pursue this review that you Abandon it.
>
>
> Sure, I already wanted that, but I don't see any options that would result in 
> closing the issue. Could you, please, point me to it? Tnx!


Scroll to the bottom of the review to the "Add Action..." combo box, then 
select "Abandon Revision"

F8324493: image.png 


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58178



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

[PATCH] D58495: [clangd] Only report explicitly typed symbols during code navigation

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a reviewer: hokein.
ilya-biryukov accepted this revision.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

Haojian is the author, so adding him as a reviewer. LGTM from my side




Comment at: unittests/clangd/XRefsTests.cpp:416
+void foo() {
+  auto x = m^akeX();
+}

Does that mean we're not showing constructor results in:
```
X foo^(something);
```


It's the only thing that I would miss. OTOH, I also think it should be attached 
to the parentheses and it's not the case now.

Anyhow, not a huge deal.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58495



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

[PATCH] D58404: [clang-format] Add basic support for formatting C# files

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 187748.
MyDeveloperDay added a comment.

Add to release notes


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

https://reviews.llvm.org/D58404

Files:
  docs/ClangFormat.rst
  docs/ClangFormatStyleOptions.rst
  docs/ReleaseNotes.rst
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  tools/clang-format/ClangFormat.cpp
  unittests/Format/CMakeLists.txt
  unittests/Format/FormatTestCSharp.cpp

Index: unittests/Format/FormatTestCSharp.cpp
===
--- /dev/null
+++ unittests/Format/FormatTestCSharp.cpp
@@ -0,0 +1,100 @@
+//===- unittest/Format/FormatTestCSharp.cpp - Formatting tests for CSharp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+
+class FormatTestCSharp : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle &Style) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(Offset, Length));
+tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string
+  format(llvm::StringRef Code,
+ const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_CSharp)) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static FormatStyle getStyleWithColumns(unsigned ColumnLimit) {
+FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+Style.ColumnLimit = ColumnLimit;
+return Style;
+  }
+
+  static void verifyFormat(
+  llvm::StringRef Code,
+  const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_CSharp)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
+  }
+};
+
+TEST_F(FormatTestCSharp, CSharpClass) {
+  verifyFormat("public class SomeClass {\n"
+   "  void f() {}\n"
+   "  int g() { return 0; }\n"
+   "  void h() {\n"
+   "while (true) f();\n"
+   "for (;;) f();\n"
+   "if (true) f();\n"
+   "  }\n"
+   "}");
+}
+
+TEST_F(FormatTestCSharp, AccessModifiers) {
+  verifyFormat("public String toString() {}");
+  verifyFormat("private String toString() {}");
+  verifyFormat("protected String toString() {}");
+  verifyFormat("internal String toString() {}");
+
+  verifyFormat("public override String toString() {}");
+  verifyFormat("private override String toString() {}");
+  verifyFormat("protected override String toString() {}");
+  verifyFormat("internal override String toString() {}");
+
+  verifyFormat("internal static String toString() {}");
+}
+
+TEST_F(FormatTestCSharp, NoStringLiteralBreaks) {
+  verifyFormat("foo("
+   "\"a"
+   "aa\");");
+}
+
+TEST_F(FormatTestCSharp, Attributes) {
+  verifyFormat("[STAThread]\n"
+   "static void\n"
+   "Main(string[] args) {}");
+
+  verifyFormat("[TestMethod]\n"
+   "[DeploymentItem(\"Test.txt\")]\n"
+   "public class Test {}");
+
+  verifyFormat("[System.AttributeUsage(System.AttributeTargets.Method)]\n"
+   "[System.Runtime.InteropServices.ComVisible(true)]\n"
+   "public sealed class STAThreadAttribute : Attribute {}");
+}
+
+} // namespace format
+} // end namespace clang
Index: unittests/Format/CMakeLists.txt
===
--- unittests/Format/CMakeLists.txt
+++ unittests/Format/CMakeLists.txt
@@ -6,6 +6,7 @@
   CleanupTest.cpp
   FormatTest.cpp
   FormatTestComments.cpp
+  FormatTestCSharp.cpp
   FormatTestJS.cpp
   FormatTestJava.cpp
   FormatTestObjC.cpp
Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -345,7 +345,7 @@
   cl::SetVersionPrinter(PrintVersion);
   cl::ParseCommandLineOptions(
   argc, argv,
-  "A too

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 187751.
bader added a comment.

Applied comments from @ABataev.

- Split changes into two patches. This part contains front-end option enabling 
device specific macro. Changes adding driver option will be sent in a separate 
patch.
- Added LIT test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/sycl-macro.cpp


Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck %s
+
+// CHECK:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1057,6 +1057,12 @@
 Builder.defineMacro("__CLANG_CUDA_APPROX_TRANSCENDENTALS__");
   }
 
+  // Define indicating that the source file is being compiled with a SYCL
+  // device compiler which doesn't produce host binary.
+  if (LangOpts.SYCLIsDevice) {
+Builder.defineMacro("__SYCL_DEVICE_ONLY__", "1");
+  }
+
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
 #define OPENCLEXT(Ext) 
\
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2879,6 +2879,8 @@
   << Opts.OMPHostIRFile;
   }
 
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+
   // Set CUDA mode for OpenMP target NVPTX if specified in options
   Opts.OpenMPCUDAMode = Opts.OpenMPIsDevice && T.isNVPTX() &&
 Args.hasArg(options::OPT_fopenmp_cuda_mode);
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -836,8 +836,14 @@
 def fopenmp_host_ir_file_path : Separate<["-"], "fopenmp-host-ir-file-path">,
   HelpText<"Path to the IR file produced by the frontend for the host.">;
 
-} // let Flags = [CC1Option]
+//===--===//
+// SYCL Options
+//===--===//
 
+def fsycl_is_device : Flag<["-"], "fsycl-is-device">,
+  HelpText<"Generate code for SYCL device.">;
+
+} // let Flags = [CC1Option]
 
 
//===--===//
 // cc1as-only Options
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -217,6 +217,8 @@
 LANGOPT(CUDADeviceApproxTranscendentals, 1, 0, "using approximate 
transcendental functions")
 LANGOPT(GPURelocatableDeviceCode, 1, 0, "generate relocatable device code")
 
+LANGOPT(SYCLIsDevice  , 1, 0, "Generate code for SYCL device")
+
 LANGOPT(SizedDeallocation , 1, 0, "sized deallocation")
 LANGOPT(AlignedAllocation , 1, 0, "aligned allocation")
 LANGOPT(AlignedAllocationUnavailable, 1, 0, "aligned allocation functions are 
unavailable")


Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck %s
+
+// CHECK:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1057,6 +1057,12 @@
 Builder.defineMacro("__CLANG_CUDA_APPROX_TRANSCENDENTALS__");
   }
 
+  // Define indicating that the source file is being compiled with a SYCL
+  // device compiler which doesn't produce host binary.
+  if (LangOpts.SYCLIsDevice) {
+Builder.defineMacro("__SYCL_DEVICE_ONLY__", "1");
+  }
+
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
 #define OPENCLEXT(Ext) \
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2879,6 +2879,8 @@
   << Opts.OMPHostIRFile;
   }
 
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+
   // Set CUDA mode for OpenMP target NVPTX 

r354568 - [OpenCL] Simplify LLVM IR generated for OpenCL blocks

[Andrew Savonichev via cfe-commits]

Author: asavonic
Date: Thu Feb 21 03:02:10 2019
New Revision: 354568

URL: http://llvm.org/viewvc/llvm-project?rev=354568&view=rev
Log:
[OpenCL] Simplify LLVM IR generated for OpenCL blocks

Summary:
Emit direct call of block invoke functions when possible, i.e. in case the
block is not passed as a function argument.
Also doing some refactoring of `CodeGenFunction::EmitBlockCallExpr()`

Reviewers: Anastasia, yaxunl, svenvh

Reviewed By: Anastasia

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenCLRuntime.h
cfe/trunk/test/CodeGenOpenCL/blocks.cl
cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=354568&r1=354567&r2=354568&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Thu Feb 21 03:02:10 2019
@@ -1253,52 +1253,49 @@ RValue CodeGenFunction::EmitBlockCallExp
   ReturnValueSlot ReturnValue) {
   const BlockPointerType *BPT =
 E->getCallee()->getType()->getAs();
-
   llvm::Value *BlockPtr = EmitScalarExpr(E->getCallee());
-
-  // Get a pointer to the generic block literal.
-  // For OpenCL we generate generic AS void ptr to be able to reuse the same
-  // block definition for blocks with captures generated as private AS local
-  // variables and without captures generated as global AS program scope
-  // variables.
-  unsigned AddrSpace = 0;
-  if (getLangOpts().OpenCL)
-AddrSpace = getContext().getTargetAddressSpace(LangAS::opencl_generic);
-
-  llvm::Type *BlockLiteralTy =
-  llvm::PointerType::get(CGM.getGenericBlockLiteralType(), AddrSpace);
-
-  // Bitcast the callee to a block literal.
-  BlockPtr =
-  Builder.CreatePointerCast(BlockPtr, BlockLiteralTy, "block.literal");
-
-  // Get the function pointer from the literal.
-  llvm::Value *FuncPtr =
-  Builder.CreateStructGEP(CGM.getGenericBlockLiteralType(), BlockPtr,
-  CGM.getLangOpts().OpenCL ? 2 : 3);
-
-  // Add the block literal.
+  llvm::Type *GenBlockTy = CGM.getGenericBlockLiteralType();
+  llvm::Value *Func = nullptr;
+  QualType FnType = BPT->getPointeeType();
+  ASTContext &Ctx = getContext();
   CallArgList Args;
 
-  QualType VoidPtrQualTy = getContext().VoidPtrTy;
-  llvm::Type *GenericVoidPtrTy = VoidPtrTy;
   if (getLangOpts().OpenCL) {
-GenericVoidPtrTy = CGM.getOpenCLRuntime().getGenericVoidPointerType();
-VoidPtrQualTy =
-getContext().getPointerType(getContext().getAddrSpaceQualType(
-getContext().VoidTy, LangAS::opencl_generic));
-  }
+// For OpenCL, BlockPtr is already casted to generic block literal.
 
-  BlockPtr = Builder.CreatePointerCast(BlockPtr, GenericVoidPtrTy);
-  Args.add(RValue::get(BlockPtr), VoidPtrQualTy);
+// First argument of a block call is a generic block literal casted to
+// generic void pointer, i.e. i8 addrspace(4)*
+llvm::Value *BlockDescriptor = Builder.CreatePointerCast(
+BlockPtr, CGM.getOpenCLRuntime().getGenericVoidPointerType());
+QualType VoidPtrQualTy = Ctx.getPointerType(
+Ctx.getAddrSpaceQualType(Ctx.VoidTy, LangAS::opencl_generic));
+Args.add(RValue::get(BlockDescriptor), VoidPtrQualTy);
+// And the rest of the arguments.
+EmitCallArgs(Args, FnType->getAs(), E->arguments());
+
+// We *can* call the block directly unless it is a function argument.
+if (!isa(E->getCalleeDecl()))
+  Func = CGM.getOpenCLRuntime().getInvokeFunction(E->getCallee());
+else {
+  llvm::Value *FuncPtr = Builder.CreateStructGEP(GenBlockTy, BlockPtr, 2);
+  Func = Builder.CreateAlignedLoad(FuncPtr, getPointerAlign());
+}
+  } else {
+// Bitcast the block literal to a generic block literal.
+BlockPtr = Builder.CreatePointerCast(
+BlockPtr, llvm::PointerType::get(GenBlockTy, 0), "block.literal");
+// Get pointer to the block invoke function
+llvm::Value *FuncPtr = Builder.CreateStructGEP(GenBlockTy, BlockPtr, 3);
+
+// First argument is a block literal casted to a void pointer
+BlockPtr = Builder.CreatePointerCast(BlockPtr, VoidPtrTy);
+Args.add(RValue::get(BlockPtr), Ctx.VoidPtrTy);
+// And the rest of the arguments.
+EmitCallArgs(Args, FnType->getAs(), E->arguments());
 
-  QualType FnType = BPT->getPointeeType();
-
-  // And the rest of the arguments.
-  EmitCallArgs(Args, FnType->getAs(), E->arguments());
-
-  // Load the function.
-  llvm::Value *Func = Builder.CreateAlignedLoad(FuncPtr, getPointerAlign());
+// Load the function.
+Func = Builder.CreateAlignedLoad(FuncPtr, getPointerAlign());
+  }
 
   const FunctionType *FuncTy = FnType->castAs();
 

[PATCH] D58446: [CodeComplete] Collect visited contexts when scope specifier is invalid.

[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:
  rC Clang

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

https://reviews.llvm.org/D58446



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

[PATCH] D58388: [OpenCL] Simplify LLVM IR generated for OpenCL blocks

[Andrew Savonichev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC354568: [OpenCL] Simplify LLVM IR generated for OpenCL 
blocks (authored by asavonic, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D58388

Files:
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGOpenCLRuntime.cpp
  lib/CodeGen/CGOpenCLRuntime.h
  test/CodeGenOpenCL/blocks.cl
  test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Index: lib/CodeGen/CGOpenCLRuntime.h
===
--- lib/CodeGen/CGOpenCLRuntime.h
+++ lib/CodeGen/CGOpenCLRuntime.h
@@ -91,6 +91,10 @@
   /// \param Block block literal emitted for the block expression.
   void recordBlockInfo(const BlockExpr *E, llvm::Function *InvokeF,
llvm::Value *Block);
+
+  /// \return LLVM block invoke function emitted for an expression derived from
+  /// the block expression.
+  llvm::Function *getInvokeFunction(const Expr *E);
 };
 
 }
Index: lib/CodeGen/CGBlocks.cpp
===
--- lib/CodeGen/CGBlocks.cpp
+++ lib/CodeGen/CGBlocks.cpp
@@ -1253,52 +1253,49 @@
   ReturnValueSlot ReturnValue) {
   const BlockPointerType *BPT =
 E->getCallee()->getType()->getAs();
-
   llvm::Value *BlockPtr = EmitScalarExpr(E->getCallee());
-
-  // Get a pointer to the generic block literal.
-  // For OpenCL we generate generic AS void ptr to be able to reuse the same
-  // block definition for blocks with captures generated as private AS local
-  // variables and without captures generated as global AS program scope
-  // variables.
-  unsigned AddrSpace = 0;
-  if (getLangOpts().OpenCL)
-AddrSpace = getContext().getTargetAddressSpace(LangAS::opencl_generic);
-
-  llvm::Type *BlockLiteralTy =
-  llvm::PointerType::get(CGM.getGenericBlockLiteralType(), AddrSpace);
-
-  // Bitcast the callee to a block literal.
-  BlockPtr =
-  Builder.CreatePointerCast(BlockPtr, BlockLiteralTy, "block.literal");
-
-  // Get the function pointer from the literal.
-  llvm::Value *FuncPtr =
-  Builder.CreateStructGEP(CGM.getGenericBlockLiteralType(), BlockPtr,
-  CGM.getLangOpts().OpenCL ? 2 : 3);
-
-  // Add the block literal.
+  llvm::Type *GenBlockTy = CGM.getGenericBlockLiteralType();
+  llvm::Value *Func = nullptr;
+  QualType FnType = BPT->getPointeeType();
+  ASTContext &Ctx = getContext();
   CallArgList Args;
 
-  QualType VoidPtrQualTy = getContext().VoidPtrTy;
-  llvm::Type *GenericVoidPtrTy = VoidPtrTy;
   if (getLangOpts().OpenCL) {
-GenericVoidPtrTy = CGM.getOpenCLRuntime().getGenericVoidPointerType();
-VoidPtrQualTy =
-getContext().getPointerType(getContext().getAddrSpaceQualType(
-getContext().VoidTy, LangAS::opencl_generic));
-  }
-
-  BlockPtr = Builder.CreatePointerCast(BlockPtr, GenericVoidPtrTy);
-  Args.add(RValue::get(BlockPtr), VoidPtrQualTy);
-
-  QualType FnType = BPT->getPointeeType();
+// For OpenCL, BlockPtr is already casted to generic block literal.
 
-  // And the rest of the arguments.
-  EmitCallArgs(Args, FnType->getAs(), E->arguments());
+// First argument of a block call is a generic block literal casted to
+// generic void pointer, i.e. i8 addrspace(4)*
+llvm::Value *BlockDescriptor = Builder.CreatePointerCast(
+BlockPtr, CGM.getOpenCLRuntime().getGenericVoidPointerType());
+QualType VoidPtrQualTy = Ctx.getPointerType(
+Ctx.getAddrSpaceQualType(Ctx.VoidTy, LangAS::opencl_generic));
+Args.add(RValue::get(BlockDescriptor), VoidPtrQualTy);
+// And the rest of the arguments.
+EmitCallArgs(Args, FnType->getAs(), E->arguments());
+
+// We *can* call the block directly unless it is a function argument.
+if (!isa(E->getCalleeDecl()))
+  Func = CGM.getOpenCLRuntime().getInvokeFunction(E->getCallee());
+else {
+  llvm::Value *FuncPtr = Builder.CreateStructGEP(GenBlockTy, BlockPtr, 2);
+  Func = Builder.CreateAlignedLoad(FuncPtr, getPointerAlign());
+}
+  } else {
+// Bitcast the block literal to a generic block literal.
+BlockPtr = Builder.CreatePointerCast(
+BlockPtr, llvm::PointerType::get(GenBlockTy, 0), "block.literal");
+// Get pointer to the block invoke function
+llvm::Value *FuncPtr = Builder.CreateStructGEP(GenBlockTy, BlockPtr, 3);
+
+// First argument is a block literal casted to a void pointer
+BlockPtr = Builder.CreatePointerCast(BlockPtr, VoidPtrTy);
+Args.add(RValue::get(BlockPtr), Ctx.VoidPtrTy);
+// And the rest of the arguments.
+EmitCallArgs(Args, FnType->getAs(), E->arguments());
 
-  // Load the function.
-  llvm::Value *Func = Builder.CreateAlignedLoad(FuncPtr, getPointerAlign());
+// Load the function.
+Func = Builder.CreateAlignedLoad(FuncPtr, getPointerAlign());
+  }
 
   const FunctionType *FuncTy = Fn

[PATCH] D58446: [CodeComplete] Collect visited contexts when scope specifier is invalid.

[Eric Liu via Phabricator via cfe-commits]

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

- Cleanup unit tests (forgot to run... sorry)


Repository:
  rC Clang

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

https://reviews.llvm.org/D58446

Files:
  lib/Sema/SemaCodeComplete.cpp
  unittests/Sema/CodeCompleteTest.cpp


Index: unittests/Sema/CodeCompleteTest.cpp
===
--- unittests/Sema/CodeCompleteTest.cpp
+++ unittests/Sema/CodeCompleteTest.cpp
@@ -173,12 +173,16 @@
   "foo::(anonymous)"));
 }
 
-TEST(SemaCodeCompleteTest, VisitedNSForInvalideQualifiedId) {
+TEST(SemaCodeCompleteTest, VisitedNSForInvalidQualifiedId) {
   auto VisitedNS = runCodeCompleteOnCode(R"cpp(
- namespace ns { foo::^ }
+ namespace na {}
+ namespace ns1 {
+ using namespace na;
+ foo::^
+ }
   )cpp")
.VisitedNamespaces;
-  EXPECT_TRUE(VisitedNS.empty());
+  EXPECT_THAT(VisitedNS, UnorderedElementsAre("ns1", "na"));
 }
 
 TEST(SemaCodeCompleteTest, VisitedNSWithoutQualifier) {
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -5061,7 +5061,20 @@
   if (SS.isInvalid()) {
 CodeCompletionContext CC(CodeCompletionContext::CCC_Symbol);
 CC.setCXXScopeSpecifier(SS);
-HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
+// As SS is invalid, we try to collect accessible contexts from the current
+// scope with a dummy lookup so that the completion consumer can try to
+// guess what the specified scope is.
+ResultBuilder DummyResults(*this, CodeCompleter->getAllocator(),
+   CodeCompleter->getCodeCompletionTUInfo(), CC);
+if (S->getEntity()) {
+  CodeCompletionDeclConsumer Consumer(DummyResults, S->getEntity(),
+  BaseType);
+  LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
+ /*IncludeGlobalScope=*/false,
+ /*LoadExternal=*/false);
+}
+HandleCodeCompleteResults(this, CodeCompleter,
+  DummyResults.getCompletionContext(), nullptr, 0);
 return;
   }
   // Always pretend to enter a context to ensure that a dependent type


Index: unittests/Sema/CodeCompleteTest.cpp
===
--- unittests/Sema/CodeCompleteTest.cpp
+++ unittests/Sema/CodeCompleteTest.cpp
@@ -173,12 +173,16 @@
   "foo::(anonymous)"));
 }
 
-TEST(SemaCodeCompleteTest, VisitedNSForInvalideQualifiedId) {
+TEST(SemaCodeCompleteTest, VisitedNSForInvalidQualifiedId) {
   auto VisitedNS = runCodeCompleteOnCode(R"cpp(
- namespace ns { foo::^ }
+ namespace na {}
+ namespace ns1 {
+ using namespace na;
+ foo::^
+ }
   )cpp")
.VisitedNamespaces;
-  EXPECT_TRUE(VisitedNS.empty());
+  EXPECT_THAT(VisitedNS, UnorderedElementsAre("ns1", "na"));
 }
 
 TEST(SemaCodeCompleteTest, VisitedNSWithoutQualifier) {
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -5061,7 +5061,20 @@
   if (SS.isInvalid()) {
 CodeCompletionContext CC(CodeCompletionContext::CCC_Symbol);
 CC.setCXXScopeSpecifier(SS);
-HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
+// As SS is invalid, we try to collect accessible contexts from the current
+// scope with a dummy lookup so that the completion consumer can try to
+// guess what the specified scope is.
+ResultBuilder DummyResults(*this, CodeCompleter->getAllocator(),
+   CodeCompleter->getCodeCompletionTUInfo(), CC);
+if (S->getEntity()) {
+  CodeCompletionDeclConsumer Consumer(DummyResults, S->getEntity(),
+  BaseType);
+  LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
+ /*IncludeGlobalScope=*/false,
+ /*LoadExternal=*/false);
+}
+HandleCodeCompleteResults(this, CodeCompleter,
+  DummyResults.getCompletionContext(), nullptr, 0);
 return;
   }
   // Always pretend to enter a context to ensure that a dependent type
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58446: [CodeComplete] Collect visited contexts when scope specifier is invalid.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354570: [CodeComplete] Collect visited contexts when scope 
specifier is invalid. (authored by ioeric, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58446

Files:
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/unittests/Sema/CodeCompleteTest.cpp


Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -5061,7 +5061,20 @@
   if (SS.isInvalid()) {
 CodeCompletionContext CC(CodeCompletionContext::CCC_Symbol);
 CC.setCXXScopeSpecifier(SS);
-HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
+// As SS is invalid, we try to collect accessible contexts from the current
+// scope with a dummy lookup so that the completion consumer can try to
+// guess what the specified scope is.
+ResultBuilder DummyResults(*this, CodeCompleter->getAllocator(),
+   CodeCompleter->getCodeCompletionTUInfo(), CC);
+if (S->getEntity()) {
+  CodeCompletionDeclConsumer Consumer(DummyResults, S->getEntity(),
+  BaseType);
+  LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
+ /*IncludeGlobalScope=*/false,
+ /*LoadExternal=*/false);
+}
+HandleCodeCompleteResults(this, CodeCompleter,
+  DummyResults.getCompletionContext(), nullptr, 0);
 return;
   }
   // Always pretend to enter a context to ensure that a dependent type
Index: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
===
--- cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
+++ cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
@@ -173,12 +173,16 @@
   "foo::(anonymous)"));
 }
 
-TEST(SemaCodeCompleteTest, VisitedNSForInvalideQualifiedId) {
+TEST(SemaCodeCompleteTest, VisitedNSForInvalidQualifiedId) {
   auto VisitedNS = runCodeCompleteOnCode(R"cpp(
- namespace ns { foo::^ }
+ namespace na {}
+ namespace ns1 {
+ using namespace na;
+ foo::^
+ }
   )cpp")
.VisitedNamespaces;
-  EXPECT_TRUE(VisitedNS.empty());
+  EXPECT_THAT(VisitedNS, UnorderedElementsAre("ns1", "na"));
 }
 
 TEST(SemaCodeCompleteTest, VisitedNSWithoutQualifier) {


Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -5061,7 +5061,20 @@
   if (SS.isInvalid()) {
 CodeCompletionContext CC(CodeCompletionContext::CCC_Symbol);
 CC.setCXXScopeSpecifier(SS);
-HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
+// As SS is invalid, we try to collect accessible contexts from the current
+// scope with a dummy lookup so that the completion consumer can try to
+// guess what the specified scope is.
+ResultBuilder DummyResults(*this, CodeCompleter->getAllocator(),
+   CodeCompleter->getCodeCompletionTUInfo(), CC);
+if (S->getEntity()) {
+  CodeCompletionDeclConsumer Consumer(DummyResults, S->getEntity(),
+  BaseType);
+  LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
+ /*IncludeGlobalScope=*/false,
+ /*LoadExternal=*/false);
+}
+HandleCodeCompleteResults(this, CodeCompleter,
+  DummyResults.getCompletionContext(), nullptr, 0);
 return;
   }
   // Always pretend to enter a context to ensure that a dependent type
Index: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
===
--- cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
+++ cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
@@ -173,12 +173,16 @@
   "foo::(anonymous)"));
 }
 
-TEST(SemaCodeCompleteTest, VisitedNSForInvalideQualifiedId) {
+TEST(SemaCodeCompleteTest, VisitedNSForInvalidQualifiedId) {
   auto VisitedNS = runCodeCompleteOnCode(R"cpp(
- namespace ns { foo::^ }
+ namespace na {}
+ namespace ns1 {
+ using namespace na;
+ foo::^
+ }
   )cpp")
.VisitedNamespaces;
-  EXPECT_TRUE(VisitedNS.empty());
+  EXPECT_THAT(VisitedNS, UnorderedElementsAre("ns1", "na"));
 }
 
 TEST(SemaCodeCompleteTest, VisitedNSWithoutQualifier) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r354570 - [CodeComplete] Collect visited contexts when scope specifier is invalid.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Thu Feb 21 03:22:58 2019
New Revision: 354570

URL: http://llvm.org/viewvc/llvm-project?rev=354570&view=rev
Log:
[CodeComplete] Collect visited contexts when scope specifier is invalid.

Summary:
This will allow completion consumers to guess the specified scope by
putting together scopes in the context with the specified scope (e.g. when the
specified namespace is not imported yet).

Reviewers: ilya-biryukov

Subscribers: jdoerfert, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/unittests/Sema/CodeCompleteTest.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=354570&r1=354569&r2=354570&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu Feb 21 03:22:58 2019
@@ -5061,7 +5061,20 @@ void Sema::CodeCompleteQualifiedId(Scope
   if (SS.isInvalid()) {
 CodeCompletionContext CC(CodeCompletionContext::CCC_Symbol);
 CC.setCXXScopeSpecifier(SS);
-HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
+// As SS is invalid, we try to collect accessible contexts from the current
+// scope with a dummy lookup so that the completion consumer can try to
+// guess what the specified scope is.
+ResultBuilder DummyResults(*this, CodeCompleter->getAllocator(),
+   CodeCompleter->getCodeCompletionTUInfo(), CC);
+if (S->getEntity()) {
+  CodeCompletionDeclConsumer Consumer(DummyResults, S->getEntity(),
+  BaseType);
+  LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
+ /*IncludeGlobalScope=*/false,
+ /*LoadExternal=*/false);
+}
+HandleCodeCompleteResults(this, CodeCompleter,
+  DummyResults.getCompletionContext(), nullptr, 0);
 return;
   }
   // Always pretend to enter a context to ensure that a dependent type

Modified: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Sema/CodeCompleteTest.cpp?rev=354570&r1=354569&r2=354570&view=diff
==
--- cfe/trunk/unittests/Sema/CodeCompleteTest.cpp (original)
+++ cfe/trunk/unittests/Sema/CodeCompleteTest.cpp Thu Feb 21 03:22:58 2019
@@ -173,12 +173,16 @@ TEST(SemaCodeCompleteTest, VisitedNSForV
   "foo::(anonymous)"));
 }
 
-TEST(SemaCodeCompleteTest, VisitedNSForInvalideQualifiedId) {
+TEST(SemaCodeCompleteTest, VisitedNSForInvalidQualifiedId) {
   auto VisitedNS = runCodeCompleteOnCode(R"cpp(
- namespace ns { foo::^ }
+ namespace na {}
+ namespace ns1 {
+ using namespace na;
+ foo::^
+ }
   )cpp")
.VisitedNamespaces;
-  EXPECT_TRUE(VisitedNS.empty());
+  EXPECT_THAT(VisitedNS, UnorderedElementsAre("ns1", "na"));
 }
 
 TEST(SemaCodeCompleteTest, VisitedNSWithoutQualifier) {


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

[PATCH] D58497: Clear the KnownModules cache if the preprocessor is going away

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai created this revision.
nemanjai added a reviewer: rsmith.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

When the `Preprocessor (PP)` in compiler instance is going away, we should 
clear the cache of any pointers that it owns as they will be destroyed.

This is one possible fix for the issue I outlined in 
http://lists.llvm.org/pipermail/cfe-dev/2019-February/061293.html that received 
no responses. We have now encountered the issue in multiple internal buildbots 
and I have even seen it in external bots as well. This really should be fixed.

As outlined in my cfe-dev post, it is exceedingly difficult to produce a 
reliable test case for this (at least for me) so I have not provided one.

If others should be on the list of reviewers, please add them.


Repository:
  rC Clang

https://reviews.llvm.org/D58497

Files:
  lib/Frontend/CompilerInstance.cpp


Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -374,7 +374,14 @@
   // The module manager holds a reference to the old preprocessor (if any).
   ModuleManager.reset();
 
-  // Create the Preprocessor.
+  // Create the Preprocessor. If this instance is replacing the existing
+  // preprocessor and that existing one is going away, we have to remove
+  // the Module* pointers it owns from KnownModules since they will be
+  // dangling. FIXME: Should this only remove pointers owned by the
+  // preprocessor that is going away or clear the entire map (or can
+  // the map even own any other Module* pointers)?
+  if (PP.unique())
+KnownModules.clear();
   HeaderSearch *HeaderInfo =
   new HeaderSearch(getHeaderSearchOptsPtr(), getSourceManager(),
getDiagnostics(), getLangOpts(), &getTarget());


Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -374,7 +374,14 @@
   // The module manager holds a reference to the old preprocessor (if any).
   ModuleManager.reset();
 
-  // Create the Preprocessor.
+  // Create the Preprocessor. If this instance is replacing the existing
+  // preprocessor and that existing one is going away, we have to remove
+  // the Module* pointers it owns from KnownModules since they will be
+  // dangling. FIXME: Should this only remove pointers owned by the
+  // preprocessor that is going away or clear the entire map (or can
+  // the map even own any other Module* pointers)?
+  if (PP.unique())
+KnownModules.clear();
   HeaderSearch *HeaderInfo =
   new HeaderSearch(getHeaderSearchOptsPtr(), getSourceManager(),
getDiagnostics(), getLangOpts(), &getTarget());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58495: [clangd] Only report explicitly typed symbols during code navigation

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

+1 on removing the implicit references, sometimes it gives very confusing 
results.




Comment at: clangd/XRefs.cpp:113
 
 struct DeclInfo {
   const Decl *D;

This structure is not used anymore, remove it.



Comment at: clangd/XRefs.cpp:124
   // explicitly in the code.
   // True means the declaration is explicitly referenced at least once; false
   // otherwise.

The comment is stale.



Comment at: unittests/clangd/XRefsTests.cpp:465
   $4^g($5^f());
   g($6^str);
 }

Can we add one more testcase?

```
Foo ab^c;
```



Comment at: unittests/clangd/XRefsTests.cpp:469
   auto AST = TestTU::withCode(T.code()).build();
   // Ordered assertions are deliberate: we expect a predictable order.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("1")), ElementsAre(Sym("str")));

The comment is stale.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58495



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

[PATCH] D58501: [libclang] Fix CXTranslationUnit_KeepGoing

[Nikolai Kosjar via Phabricator via cfe-commits]

nik created this revision.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.

Since

  commit 56f5487e4387a69708f70724d00e9e076153
  [modules] Round-trip -Werror flag through explicit module build.

the behavior of CXTranslationUnit_KeepGoing changed:
Unresolved #includes are fatal errors again. As a consequence, some
templates are not instantiated and lead to confusing errors.

Revert to the old behavior: With CXTranslationUnit_KeepGoing fatal
errors are mapped to errors.


Repository:
  rC Clang

https://reviews.llvm.org/D58501

Files:
  include/clang/Basic/Diagnostic.h
  lib/Basic/DiagnosticIDs.cpp
  test/Index/Inputs/keep-going-template-instantiations.h
  test/Index/keep-going-template-instantiations.cpp
  test/Index/keep-going.cpp
  tools/libclang/CIndex.cpp
  unittests/Basic/DiagnosticTest.cpp

Index: unittests/Basic/DiagnosticTest.cpp
===
--- unittests/Basic/DiagnosticTest.cpp
+++ unittests/Basic/DiagnosticTest.cpp
@@ -46,13 +46,13 @@
   EXPECT_FALSE(Diags.hasUnrecoverableErrorOccurred());
 }
 
-// Check that SuppressAfterFatalError works as intended
-TEST(DiagnosticTest, suppressAfterFatalError) {
-  for (unsigned Suppress = 0; Suppress != 2; ++Suppress) {
+// Check that FatalsAsError works as intended
+TEST(DiagnosticTest, fatalAsError) {
+  for (unsigned FatalAsError = 0; FatalAsError != 2; ++FatalAsError) {
 DiagnosticsEngine Diags(new DiagnosticIDs(),
 new DiagnosticOptions,
 new IgnoringDiagConsumer());
-Diags.setSuppressAfterFatalError(Suppress);
+Diags.setFatalsAsError(FatalAsError);
 
 // Diag that would set UnrecoverableErrorOccurred and ErrorOccurred.
 Diags.Report(diag::err_cannot_open_file) << "file" << "error";
@@ -62,13 +62,13 @@
 Diags.Report(diag::warn_mt_message) << "warning";
 
 EXPECT_TRUE(Diags.hasErrorOccurred());
-EXPECT_TRUE(Diags.hasFatalErrorOccurred());
+EXPECT_EQ(Diags.hasFatalErrorOccurred(), FatalAsError ? 0u : 1u);
 EXPECT_TRUE(Diags.hasUncompilableErrorOccurred());
 EXPECT_TRUE(Diags.hasUnrecoverableErrorOccurred());
 
 // The warning should be emitted and counted only if we're not suppressing
 // after fatal errors.
-EXPECT_EQ(Diags.getNumWarnings(), Suppress ? 0u : 1u);
+EXPECT_EQ(Diags.getNumWarnings(), FatalAsError);
   }
 }
 
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3409,7 +3409,7 @@
 Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions));
 
   if (options & CXTranslationUnit_KeepGoing)
-Diags->setSuppressAfterFatalError(false);
+Diags->setFatalsAsError(true);
 
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar
+
+// RUN: env CINDEXTEST_KEEP_GOING=1 c-index-test -test-load-source none -I%S/Inputs %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: expected class name
Index: test/Index/Inputs/keep-going-template-instantiations.h
===
--- /dev/null
+++ test/Index/Inputs/keep-going-template-instantiations.h
@@ -0,0 +1,3 @@
+template struct c {};
+using d = c;
+struct foo : public d {};
Index: lib/Basic/DiagnosticIDs.cpp
===
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -481,6 +481,11 @@
   Result = diag::Severity::Fatal;
   }
 
+  // If explicitly requested, map fatal errors to errors.
+  if (Result == diag::Severity::Fatal &&
+  Diag.CurDiagID != diag::fatal_too_many_errors && Diag.FatalsAsError)
+Result = diag::Severity::Error;
+
   // Custom diagnostics always are emitted in system headers.
   bool ShowInSystemHeader =
   !GetDiagInfo(DiagID) || GetDiagInfo(DiagID)->WarnShowInSystemHeader;
@@ -660,7 +665,7 @@
 
   // If a fatal error has already been emitted, silence all subsequent
   // diagnostics.
-  if (Diag.FatalErrorOccurred && Diag.SuppressAfterFatalError) {
+  if (Diag.FatalErrorOccurred) {
 if (DiagLevel >= DiagnosticIDs::Error &&
 Diag.Client->IncludeInDiagnosticCounts()) {
   ++Diag.NumErrors;
Index: include/clang/Basic/Diagnostic.h
===
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -209,8 +209,8 @@
   // Used by __extension__
   unsigned char AllExtensionsSilenced = 0;
 
-  // Suppress diagnostics after a fatal error?
-  bool SuppressAfterFatalError = true;
+  // Treat fatal errors like errors.
+  bool FatalsAsError = false;
 
   // Suppress all diagnostics.
   bool SuppressAllDiagnostics = false;
@@ -614,9 +614,11 @@
   void setErrorsAsFatal(bool Val) { GetCurDiagState()->ErrorsAsFatal = Val; }
   bool getErrorsAsFatal() const { return GetCurDiagState()->ErrorsAsFatal; }
 

[PATCH] D58501: [libclang] Fix CXTranslationUnit_KeepGoing

[Nikolai Kosjar via Phabricator via cfe-commits]

nik updated this revision to Diff 187765.
nik added a comment.

Fixed minor typo.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58501

Files:
  include/clang/Basic/Diagnostic.h
  lib/Basic/DiagnosticIDs.cpp
  test/Index/Inputs/keep-going-template-instantiations.h
  test/Index/keep-going-template-instantiations.cpp
  test/Index/keep-going.cpp
  tools/libclang/CIndex.cpp
  unittests/Basic/DiagnosticTest.cpp

Index: unittests/Basic/DiagnosticTest.cpp
===
--- unittests/Basic/DiagnosticTest.cpp
+++ unittests/Basic/DiagnosticTest.cpp
@@ -46,13 +46,13 @@
   EXPECT_FALSE(Diags.hasUnrecoverableErrorOccurred());
 }
 
-// Check that SuppressAfterFatalError works as intended
-TEST(DiagnosticTest, suppressAfterFatalError) {
-  for (unsigned Suppress = 0; Suppress != 2; ++Suppress) {
+// Check that FatalsAsError works as intended
+TEST(DiagnosticTest, fatalsAsError) {
+  for (unsigned FatalsAsError = 0; FatalsAsError != 2; ++FatalsAsError) {
 DiagnosticsEngine Diags(new DiagnosticIDs(),
 new DiagnosticOptions,
 new IgnoringDiagConsumer());
-Diags.setSuppressAfterFatalError(Suppress);
+Diags.setFatalsAsError(FatalsAsError);
 
 // Diag that would set UnrecoverableErrorOccurred and ErrorOccurred.
 Diags.Report(diag::err_cannot_open_file) << "file" << "error";
@@ -62,13 +62,13 @@
 Diags.Report(diag::warn_mt_message) << "warning";
 
 EXPECT_TRUE(Diags.hasErrorOccurred());
-EXPECT_TRUE(Diags.hasFatalErrorOccurred());
+EXPECT_EQ(Diags.hasFatalErrorOccurred(), FatalsAsError ? 0u : 1u);
 EXPECT_TRUE(Diags.hasUncompilableErrorOccurred());
 EXPECT_TRUE(Diags.hasUnrecoverableErrorOccurred());
 
 // The warning should be emitted and counted only if we're not suppressing
 // after fatal errors.
-EXPECT_EQ(Diags.getNumWarnings(), Suppress ? 0u : 1u);
+EXPECT_EQ(Diags.getNumWarnings(), FatalsAsError);
   }
 }
 
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3409,7 +3409,7 @@
 Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions));
 
   if (options & CXTranslationUnit_KeepGoing)
-Diags->setSuppressAfterFatalError(false);
+Diags->setFatalsAsError(true);
 
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar
+
+// RUN: env CINDEXTEST_KEEP_GOING=1 c-index-test -test-load-source none -I%S/Inputs %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: expected class name
Index: test/Index/Inputs/keep-going-template-instantiations.h
===
--- /dev/null
+++ test/Index/Inputs/keep-going-template-instantiations.h
@@ -0,0 +1,3 @@
+template struct c {};
+using d = c;
+struct foo : public d {};
Index: lib/Basic/DiagnosticIDs.cpp
===
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -481,6 +481,11 @@
   Result = diag::Severity::Fatal;
   }
 
+  // If explicitly requested, map fatal errors to errors.
+  if (Result == diag::Severity::Fatal &&
+  Diag.CurDiagID != diag::fatal_too_many_errors && Diag.FatalsAsError)
+Result = diag::Severity::Error;
+
   // Custom diagnostics always are emitted in system headers.
   bool ShowInSystemHeader =
   !GetDiagInfo(DiagID) || GetDiagInfo(DiagID)->WarnShowInSystemHeader;
@@ -660,7 +665,7 @@
 
   // If a fatal error has already been emitted, silence all subsequent
   // diagnostics.
-  if (Diag.FatalErrorOccurred && Diag.SuppressAfterFatalError) {
+  if (Diag.FatalErrorOccurred) {
 if (DiagLevel >= DiagnosticIDs::Error &&
 Diag.Client->IncludeInDiagnosticCounts()) {
   ++Diag.NumErrors;
Index: include/clang/Basic/Diagnostic.h
===
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -209,8 +209,8 @@
   // Used by __extension__
   unsigned char AllExtensionsSilenced = 0;
 
-  // Suppress diagnostics after a fatal error?
-  bool SuppressAfterFatalError = true;
+  // Treat fatal errors like errors.
+  bool FatalsAsError = false;
 
   // Suppress all diagnostics.
   bool SuppressAllDiagnostics = false;
@@ -614,9 +614,11 @@
   void setErrorsAsFatal(bool Val) { GetCurDiagState()->ErrorsAsFatal = Val; }
   bool getErrorsAsFatal() const { return GetCurDiagState()->ErrorsAsFatal; }
 
-  /// When set to true (the default), suppress further diagnostics after
-  /// a fatal error.
-  void setSuppressAfterFatalError(bool Val) { SuppressAfterFatalError = Val; }
+  /// \brief When set to true, any fatal error reported is made an error.
+  ///
+  /// This setting takes precedence over the setErrorsAsFatal setting above.
+  void setFatalsAsEr

[PATCH] D58502: [ASTImporter] Fix redecl failures of Class and ClassTemplate

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: a_sidorin, shafik.
Herald added subscribers: cfe-commits, jdoerfert, gamesh411, Szelethus, dkrupp, 
rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.

Redecl chains of classes and class templates are not handled well
currently. We want to handle them similarly to functions, i.e. try to
keep the structure of the original AST as much as possible. The aim is
to not squash a prototype with a definition, rather we create both and
put them in a redecl chain.


Repository:
  rC Clang

https://reviews.llvm.org/D58502

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -4207,8 +4207,7 @@
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportDefinitionAfterImportedPrototype)
-// FIXME This does not pass, possible error with Class import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, ,
 ImportDefinitionAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Variable, ,
 ImportDefinitionAfterImportedPrototype)
@@ -4216,16 +4215,14 @@
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate,
 DISABLED_,
 ImportDefinitionAfterImportedPrototype)
-// FIXME This does not pass, possible error with ClassTemplate import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, ,
 ImportDefinitionAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
 ImportDefinitionAfterImportedPrototype)
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportPrototypeAfterImportedDefinition)
-// FIXME This does not pass, possible error with Class import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, ,
 ImportPrototypeAfterImportedDefinition)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Variable, ,
 ImportPrototypeAfterImportedDefinition)
@@ -4233,8 +4230,7 @@
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate,
 DISABLED_,
 ImportPrototypeAfterImportedDefinition)
-// FIXME This does not pass, possible error with ClassTemplate import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, ,
 ImportPrototypeAfterImportedDefinition)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
 ImportPrototypeAfterImportedDefinition)
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2552,26 +2552,6 @@
 Decl::FOK_None;
   }
 
-  // If this record has a definition in the translation unit we're coming from,
-  // but this particular declaration is not that definition, import the
-  // definition and map to that.
-  TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D &&
-  // Friend template declaration must be imported on its own.
-  !IsFriendTemplate &&
-  // In contrast to a normal CXXRecordDecl, the implicit
-  // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
-  // The definition of the implicit CXXRecordDecl in this case is the
-  // ClassTemplateSpecializationDecl itself. Thus, we start with an extra
-  // condition in order to be able to import the implict Decl.
-  !D->isImplicit()) {
-ExpectedDecl ImportedDefOrErr = import(Definition);
-if (!ImportedDefOrErr)
-  return ImportedDefOrErr.takeError();
-
-return Importer.MapImported(D, *ImportedDefOrErr);
-  }
-
   // Import the major distinguishing characteristics of this record.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
@@ -2600,7 +2580,8 @@
 auto FoundDecls =
 Importer.findDeclsInToCtx(DC, SearchName);
 if (!FoundDecls.empty()) {
-  // We're going to have to compare D against potentially conflicting Decls, so complete it.
+  // We're going to have to compare D against potentially conf

[PATCH] D58492: [clangd] Add thread priority lowering for MacOS as well

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a reviewer: gribozavr.
ilya-biryukov added a subscriber: gribozavr.
ilya-biryukov added a comment.

Not an expert on MacOS threading APIs, only checked this builds on Mac.
@gribozavr could you confirm using these APIs is the right thing to do on Mac?




Comment at: clangd/Threading.cpp:126
+  // 
https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/getpriority.2.html
+  setpriority(PRIO_DARWIN_THREAD, 0,
+  Priority == ThreadPriority::Low && !AvoidThreadStarvation

Could you add a corresponding #include directive on Mac (`#include 
`)?



Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58492



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

[PATCH] D58492: [clangd] Add thread priority lowering for MacOS as well

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Threading.cpp:125
+#elif defined(__APPLE__)
+  // 
https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/getpriority.2.html
+  setpriority(PRIO_DARWIN_THREAD, 0,

This points into `iPhoneOS` docs, might be confusing.
Is there a link for the desktop version of the OS somewhere?
Could also leave it out, I'm sure setpriority is easy to google.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58492



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

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

I think what you would really want to do is mark the masks as `inline 
constexpr`, but sadly inline variables are C++17 only. I have seen some ways to 
emulate inline variables but I am not sure whether it is worth doing so in this 
case.


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

https://reviews.llvm.org/D57914



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

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

A few more comments.




Comment at: clang-tidy/tool/clang-tidy-diff.py:58
+  with lock:
+sys.stdout.write((' '.join(command)).decode('utf-8') + '\n' + 
stdout.decode('utf-8') + '\n')
+if stderr:

nit: let's wrap this to 80 characters.



Comment at: clang-tidy/tool/clang-tidy-diff.py:133
+if args.export_fixes:
+  print("error: -export-fixes and -j are mutually exclusive.")
+  sys.exit(1)

What's wrong with -export-fixes and -j? Different clang-tidy processes could 
export the fixes to different files (e.g. the -export-fixes= value could be 
used as a prefix of the filename with a sequential or random suffix. WDYT?



Comment at: clang-tidy/tool/clang-tidy-diff.py:179
+  # Run a pool of clang-tidy workers.
+  run_workers(max_task, run_tidy, task_queue, lock, args.timeout)
 

"start_workers" would be a clearer name, IMO.


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

https://reviews.llvm.org/D57662



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

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[pierre gousseau via Phabricator via cfe-commits]

pgousseau added a comment.

In D57914#1405615 , @riccibruno wrote:

> I think what you would really want to do is mark the masks as `inline 
> constexpr`, but sadly inline variables are C++17 only. I have seen some ways 
> to emulate inline variables but I am not sure whether it is worth doing so in 
> this case.


Yes thanks for the advice.
I have tried moving the definitions to the cpp file but ran into initialization 
order fiasco issue because of SanitizerArgs.cpp global definitions.
If using a constexpr SanitizerMask ctor then that would work around the 
initialization fiasco issue hopefully?
Although I am not having much luck using constexpr ctor in VS2015 because of 
the array member, so might have to revert back to lo/hi mask members?


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

https://reviews.llvm.org/D57914



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

[PATCH] D58495: [clangd] Only report explicitly typed symbols during code navigation

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 187778.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Add more test cases
- Get rid of number of parameters check in implicitness control


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58495

Files:
  clangd/XRefs.cpp
  unittests/clangd/SymbolInfoTests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -406,6 +406,16 @@
 
 double y = va^r;
   )cpp",
+
+  R"cpp(// No implicit constructors
+class X {
+  X(X&& x) = default;
+};
+X [[makeX]]() {}
+void foo() {
+  auto x = m^akeX();
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
@@ -449,24 +459,28 @@
 void call() {
   const char* str = "123";
   Foo a = $1^str;
-  Foo b = Foo($2^str);
+  Foo b = F$9^oo($2^str);
   Foo c = $3^f();
   $4^g($5^f());
   g($6^str);
+  Foo ab$7^c;
+  Foo ab$8^cd("asdf");
 }
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
   // Ordered assertions are deliberate: we expect a predictable order.
-  EXPECT_THAT(locateSymbolAt(AST, T.point("1")),
-  ElementsAre(Sym("str"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("1")), ElementsAre(Sym("str")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("2")), ElementsAre(Sym("str")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("3")),
-  ElementsAre(Sym("f"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("3")), ElementsAre(Sym("f")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("5")),
-  ElementsAre(Sym("f"), Sym("Foo")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("6")),
-  ElementsAre(Sym("str"), Sym("Foo"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("5")), ElementsAre(Sym("f")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("6")), ElementsAre(Sym("str")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("7")), ElementsAre(Sym("abc")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("8")),
+  ElementsAre(Sym("Foo"), Sym("abcd")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("9")),
+  // First one is calss definition, second is the constructor.
+  ElementsAre(Sym("Foo"), Sym("Foo")));
 }
 
 TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -180,12 +180,8 @@
 func_baz1(f^f);
   }
 )cpp",
-  {
-  CreateExpectedSymbolDetails(
-  "ff", "func_baz2", "c:TestTU.cpp@218@F@func_baz2#@ff"),
-  CreateExpectedSymbolDetails(
-  "bar", "bar::", "c:@S@bar@F@bar#&1$@S@foo#"),
-  }},
+  {CreateExpectedSymbolDetails(
+  "ff", "func_baz2", "c:TestTU.cpp@218@F@func_baz2#@ff")}},
   {
   R"cpp( // Type reference - declaration
   struct foo;
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -110,20 +110,10 @@
   const MacroInfo *Info;
 };
 
-struct DeclInfo {
-  const Decl *D;
-  // Indicates the declaration is referenced by an explicit AST node.
-  bool IsReferencedExplicitly = false;
-};
-
 /// Finds declarations locations that a given source location refers to.
 class DeclarationAndMacrosFinder : public index::IndexDataConsumer {
   std::vector MacroInfos;
-  // The value of the map indicates whether the declaration has been referenced
-  // explicitly in the code.
-  // True means the declaration is explicitly referenced at least once; false
-  // otherwise.
-  llvm::DenseMap Decls;
+  llvm::DenseSet Decls;
   const SourceLocation &SearchedLocation;
   const ASTContext *
   Preprocessor &PP;
@@ -133,22 +123,14 @@
  ASTContext &AST, Preprocessor &PP)
   : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
 
-  // Get all DeclInfo of the found declarations.
-  // The results are sorted by "IsReferencedExplicitly" and declaration
-  // location.
-  std::vector getFoundDecls() const {
-std::vector Result;
-for (auto It : Decls) {
-  Result.emplace_back();
-  Result.back().D = It.first;
-  Result.back().IsReferencedExplicitly = It.second;
-}
+  // The results are sorted by declaration location.
+  std::vector getFoundDecls() const {
+std::vector Result;
+for (const Decl *D : Decls)
+  Result.push_back(D);
 
-// Sort results. Declaratio

[PATCH] D58495: [clangd] Only report explicitly typed symbols during code navigation

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: unittests/clangd/XRefsTests.cpp:469
   auto AST = TestTU::withCode(T.code()).build();
   // Ordered assertions are deliberate: we expect a predictable order.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("1")), ElementsAre(Sym("str")));

hokein wrote:
> The comment is stale.
No more


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58495



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

[PATCH] D57768: [SYCL] Add clang front-end option to enable SYCL device compilation flow.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/Preprocessor/sycl-macro.cpp:1
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck %s
+

Add a test that without this option `__SYCL_DEVICE_ONLY__` is not defined


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D58186: Sync some doc changes ClangFormatStyleOptions.rst with doc comments in `Format.h`

[Sylvestre Ledru via Phabricator via cfe-commits]

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

Do you have permissions on the repo?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58186



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

[PATCH] D58504: [OpenCL][8.0.0 Release] Notes for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: AlexeySotkin, hans.
Herald added subscribers: jdoerfert, ebevhan, yaxunl.

https://reviews.llvm.org/D58504

Files:
  docs/ReleaseNotes.rst


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -11,7 +11,7 @@
 Introduction
 
 
-This document contains the release notes for the Clang C/C++/Objective-C
+This document contains the release notes for the Clang C/C++/Objective-C/OpenCL
 frontend, part of the LLVM Compiler Infrastructure, release 8.0.0. Here we
 describe the status of Clang in some detail, including major
 improvements from the previous release and new feature work. For the
@@ -225,10 +225,59 @@
 
 ...
 
-OpenCL C Language Changes in Clang
+OpenCL Language Changes in Clang
 --
+Misc:
+
+- Improved address space support with Clang builtins.
+
+- Improved various diagnostics for vectors with element types from extensions
+  and attribute values; duplicate address spaces.
+
+- Allow blocks to capture arrays.
+
+- Allow zero assignment and comparisons between variables of queue_t type.
+
+- Improved diagnostics of formatting specifiers and argument promotions for
+  vector types in printf.
+
+- Fixed return type of enqueued kernel and pipe builtins.
+
+- Fixed address space of clk_event_t generated in the IR.
+
+- Fixed address space when passing/returning structs.
+
+Header file fixes:
+
+- Added missing extension guards around several builtin function overloads.
+
+- Fixed serialization support when registering vendor extensions using pragmas.
+
+- Fixed OpenCL version in declarations of builtin functions with with
+  sampler-less image acesses.
+
+New vendor extensions added:
+
+- ``cl_intel_planar_yuv``
+
+- ``cl_intel_device_side_avc_motion_estimation``
+
+
+C++ for OpenCL:
+
+- Added support of address space conversions in C style casts.
+
+- Enabled address spaces for references.
+
+- Fixed use of address spaces in templates: deduction and diagnostics.
+
+- Changed default address space to work with C++ specific concepts: class 
members,
+  template parameters, etc.
+
+- Added generic address space by default to the generated hidden 'this' 
parameter.
+
+- Extend overload ranking rules for address spaces.
 
-...
 
 ABI Changes in Clang
 


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -11,7 +11,7 @@
 Introduction
 
 
-This document contains the release notes for the Clang C/C++/Objective-C
+This document contains the release notes for the Clang C/C++/Objective-C/OpenCL
 frontend, part of the LLVM Compiler Infrastructure, release 8.0.0. Here we
 describe the status of Clang in some detail, including major
 improvements from the previous release and new feature work. For the
@@ -225,10 +225,59 @@
 
 ...
 
-OpenCL C Language Changes in Clang
+OpenCL Language Changes in Clang
 --
+Misc:
+
+- Improved address space support with Clang builtins.
+
+- Improved various diagnostics for vectors with element types from extensions
+  and attribute values; duplicate address spaces.
+
+- Allow blocks to capture arrays.
+
+- Allow zero assignment and comparisons between variables of queue_t type.
+
+- Improved diagnostics of formatting specifiers and argument promotions for
+  vector types in printf.
+
+- Fixed return type of enqueued kernel and pipe builtins.
+
+- Fixed address space of clk_event_t generated in the IR.
+
+- Fixed address space when passing/returning structs.
+
+Header file fixes:
+
+- Added missing extension guards around several builtin function overloads.
+
+- Fixed serialization support when registering vendor extensions using pragmas.
+
+- Fixed OpenCL version in declarations of builtin functions with with
+  sampler-less image acesses.
+
+New vendor extensions added:
+
+- ``cl_intel_planar_yuv``
+
+- ``cl_intel_device_side_avc_motion_estimation``
+
+
+C++ for OpenCL:
+
+- Added support of address space conversions in C style casts.
+
+- Enabled address spaces for references.
+
+- Fixed use of address spaces in templates: deduction and diagnostics.
+
+- Changed default address space to work with C++ specific concepts: class members,
+  template parameters, etc.
+
+- Added generic address space by default to the generated hidden 'this' parameter.
+
+- Extend overload ranking rules for address spaces.
 
-...
 
 ABI Changes in Clang
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle updated this revision to Diff 187783.
malaperle added a comment.

Address comments.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55250

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

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -648,7 +648,25 @@
 #define MACRO 2
 #undef macro
   )cpp",
-  "#define MACRO",
+  "#define MACRO 1",
+  },
+  {
+  R"cpp(// Macro
+#define MACRO 0
+#define MACRO2 ^MACRO
+  )cpp",
+  "#define MACRO 0",
+  },
+  {
+  R"cpp(// Macro
+#define MACRO {\
+  return 0;\
+}
+int main() ^MACRO
+  )cpp",
+  R"cpp(#define MACRO {\
+  return 0;\
+})cpp",
   },
   {
   R"cpp(// Forward class declaration
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -537,13 +537,30 @@
   return H;
 }
 
-/// Generate a \p Hover object given the macro \p MacroInf.
-static Hover getHoverContents(llvm::StringRef MacroName) {
-  Hover H;
-
-  H.contents.value = "#define ";
-  H.contents.value += MacroName;
+/// Generate a \p Hover object given the macro \p MacroDecl.
+static Hover getHoverContents(MacroDecl Decl, ParsedAST &AST) {
+  SourceManager &SM = AST.getASTContext().getSourceManager();
+  std::string Definition = Decl.Name;
+
+  // Try to get the full definition, not just the name
+  SourceLocation StartLoc = Decl.Info->getDefinitionLoc();
+  SourceLocation EndLoc = Decl.Info->getDefinitionEndLoc();
+  if (EndLoc.isValid()) {
+EndLoc = Lexer::getLocForEndOfToken(EndLoc, 0, SM,
+AST.getASTContext().getLangOpts());
+bool Invalid;
+StringRef Buffer = SM.getBufferData(SM.getFileID(StartLoc), &Invalid);
+if (!Invalid) {
+  unsigned StartOffset = SM.getFileOffset(StartLoc);
+  unsigned EndOffset = SM.getFileOffset(EndLoc);
+  if (EndOffset <= Buffer.size() && StartOffset < EndOffset)
+Definition = Buffer.substr(StartOffset, EndOffset - StartOffset).str();
+}
+  }
 
+  Hover H;
+  H.contents.kind = MarkupKind::PlainText;
+  H.contents.value = "#define " + Definition;
   return H;
 }
 
@@ -667,7 +684,7 @@
   auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg);
 
   if (!Symbols.Macros.empty())
-return getHoverContents(Symbols.Macros[0].Name);
+return getHoverContents(Symbols.Macros[0], AST);
 
   if (!Symbols.Decls.empty())
 return getHoverContents(Symbols.Decls[0].D);
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -362,6 +362,10 @@
   /// Client supports CodeAction return value for textDocument/codeAction.
   /// textDocument.codeAction.codeActionLiteralSupport.
   bool CodeActionStructure = false;
+
+  /// The supported set of MarkupKinds for hover.
+  /// textDocument.hover.contentFormat.
+  bool HoverSupportsMarkdown = false;
 };
 bool fromJSON(const llvm::json::Value &, ClientCapabilities &);
 
@@ -822,6 +826,7 @@
   PlainText,
   Markdown,
 };
+bool fromJSON(const llvm::json::Value &, MarkupKind &);
 
 struct MarkupContent {
   MarkupKind kind = MarkupKind::PlainText;
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -245,6 +245,24 @@
   DocumentSymbol->getBoolean("hierarchicalDocumentSymbolSupport"))
 R.HierarchicalDocumentSymbol = *HierarchicalSupport;
 }
+if (auto *Hover = TextDocument->getObject("hover")) {
+  if (auto HoverMarkupKinds = Hover->get("contentFormat")) {
+auto *A = HoverMarkupKinds->getAsArray();
+if (!A) {
+  return false;
+}
+
+for (size_t I = 0; I < A->size(); ++I) {
+  MarkupKind KindOut;
+  if (fromJSON((*A)[I], KindOut)) {
+if (KindOut == MarkupKind::Markdown) {
+  R.HoverSupportsMarkdown = true;
+  break;
+}
+  }
+}
+  }
+}
   }
   if (auto *Workspace = O->getObject("workspace")) {
 if (auto *Symbol = Workspace->getObject("symbol")) {
@@ -616,6 +634,19 @@
   };
 }
 
+bool fromJSON(const llvm::json::Value &E, MarkupKind &Out) {
+  if (auto T = E.getAsString()) {
+if (*T == "plaintext")
+  Out = MarkupKind::PlainText;
+else if (*T == "markdown")
+  Out = MarkupKind::Markdown;
+else
+  return false;
+return true;
+  }
+  return false;
+}
+
 llvm::json::Value toJSON(const Ho

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

[Marc-Andre Laperle via Phabricator via cfe-commits]

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



Comment at: clangd/ClangdLSPServer.cpp:816
+// If the client supports Markdown, convert from plaintext here.
+if (*H && HoverSupportsMarkdown) {
+  (*H)->contents.kind = MarkupKind::Markdown;

I don't know if you meant plain text as non-language-specific markdown or no 
markdown at all. In VSCode, non-markdown for multiline macros looks bad because 
the newlines are ignored.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55250



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

[clang-tools-extra] r354585 - [clangd] Only report explicitly typed symbols during code navigation

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Feb 21 06:48:33 2019
New Revision: 354585

URL: http://llvm.org/viewvc/llvm-project?rev=354585&view=rev
Log:
[clangd] Only report explicitly typed symbols during code navigation

Summary:
Clangd was reporting implicit symbols, like results of implicit cast
expressions during code navigation, which is not desired. For example:

```
struct Foo{ Foo(int); };
void bar(Foo);
vod foo() {
  int x;
  bar(^x);
}
```
Performing a GoTo on the point specified by ^ would give two results one
pointing to line `int x` and the other for definition of `Foo(int);`

Reviewers: ilya-biryukov, sammccall

Subscribers: ioeric, MaskRay, jkorous, mgrang, arphaman, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolInfoTests.cpp
clang-tools-extra/trunk/unittests/clangd/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=354585&r1=354584&r2=354585&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Thu Feb 21 06:48:33 2019
@@ -110,20 +110,10 @@ struct MacroDecl {
   const MacroInfo *Info;
 };
 
-struct DeclInfo {
-  const Decl *D;
-  // Indicates the declaration is referenced by an explicit AST node.
-  bool IsReferencedExplicitly = false;
-};
-
 /// Finds declarations locations that a given source location refers to.
 class DeclarationAndMacrosFinder : public index::IndexDataConsumer {
   std::vector MacroInfos;
-  // The value of the map indicates whether the declaration has been referenced
-  // explicitly in the code.
-  // True means the declaration is explicitly referenced at least once; false
-  // otherwise.
-  llvm::DenseMap Decls;
+  llvm::DenseSet Decls;
   const SourceLocation &SearchedLocation;
   const ASTContext *
   Preprocessor &PP;
@@ -133,22 +123,14 @@ public:
  ASTContext &AST, Preprocessor &PP)
   : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
 
-  // Get all DeclInfo of the found declarations.
-  // The results are sorted by "IsReferencedExplicitly" and declaration
-  // location.
-  std::vector getFoundDecls() const {
-std::vector Result;
-for (auto It : Decls) {
-  Result.emplace_back();
-  Result.back().D = It.first;
-  Result.back().IsReferencedExplicitly = It.second;
-}
+  // The results are sorted by declaration location.
+  std::vector getFoundDecls() const {
+std::vector Result;
+for (const Decl *D : Decls)
+  Result.push_back(D);
 
-// Sort results. Declarations being referenced explicitly come first.
-llvm::sort(Result, [](const DeclInfo &L, const DeclInfo &R) {
-  if (L.IsReferencedExplicitly != R.IsReferencedExplicitly)
-return L.IsReferencedExplicitly > R.IsReferencedExplicitly;
-  return L.D->getBeginLoc() < R.D->getBeginLoc();
+llvm::sort(Result, [](const Decl *L, const Decl *R) {
+  return L->getBeginLoc() < R->getBeginLoc();
 });
 return Result;
   }
@@ -180,21 +162,21 @@ public:
 // clang) if it has an invalid paren/brace location, since such
 // experssion is impossible to write down.
 if (const auto *CtorExpr = dyn_cast(E))
-  return CtorExpr->getNumArgs() > 0 &&
- CtorExpr->getParenOrBraceRange().isInvalid();
+  return CtorExpr->getParenOrBraceRange().isInvalid();
 return isa(E);
   };
 
-  bool IsExplicit = !IsImplicitExpr(ASTNode.OrigE);
+  if (IsImplicitExpr(ASTNode.OrigE))
+return true;
   // Find and add definition declarations (for GoToDefinition).
   // We don't use parameter `D`, as Parameter `D` is the canonical
   // declaration, which is the first declaration of a redeclarable
   // declaration, and it could be a forward declaration.
   if (const auto *Def = getDefinition(D)) {
-Decls[Def] |= IsExplicit;
+Decls.insert(Def);
   } else {
 // Couldn't find a definition, fall back to use `D`.
-Decls[D] |= IsExplicit;
+Decls.insert(D);
   }
 }
 return true;
@@ -232,7 +214,7 @@ private:
 };
 
 struct IdentifiedSymbol {
-  std::vector Decls;
+  std::vector Decls;
   std::vector Macros;
 };
 
@@ -329,8 +311,7 @@ std::vector locateSymbolA
   llvm::DenseMap ResultIndex;
 
   // Emit all symbol locations (declaration or definition) from AST.
-  for (const DeclInfo &DI : Symbols.Decls) {
-const Decl *D = DI.D;
+  for (const Decl *D : Symbols.Decls) {
 auto Loc = makeLocation(AST, findNameLoc(D), *MainFilePath);
 if (!Loc)
   continue;
@@ -456,11 +437,7 @@ std::vector findDocum
   const SourceManager &SM = AST.getASTContext().getSourceManager();
   auto Symbols = getSymbolAtPosition(
   AST, ge

[PATCH] D58504: [OpenCL][8.0.0 Release] Notes for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia marked an inline comment as done.
Anastasia added a subscriber: ABataev.
Anastasia added inline comments.



Comment at: docs/ReleaseNotes.rst:14
 
-This document contains the release notes for the Clang C/C++/Objective-C
+This document contains the release notes for the Clang C/C++/Objective-C/OpenCL
 frontend, part of the LLVM Compiler Infrastructure, release 8.0.0. Here we

Anastasia wrote:
> @hans  I am not sure if it's best to list all languages supported (i.e. 
> OpenMP, Renderscript, CUDA are still missing) or alternatively we could 
> change to something like - Clang C language family frontend? The latter one 
> is taken from the main page: https://clang.llvm.org/. Although, OpenMP is 
> missing on the main page.
@ABataev should the main page be updated to list OpenMP explicitly?


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

https://reviews.llvm.org/D58504



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

[PATCH] D57768: [SYCL] Add clang front-end option to enable SYCL device compilation flow.

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D57768: [SYCL] Add clang front-end option to enable SYCL device compilation flow.

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 187785.
bader added a comment.

Add check that SYCL specific macro is not enabled by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/sycl-macro.cpp


Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 %s -E -dM | FileCheck %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck 
--check-prefix=CHECK-SYCL %s
+
+// CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1057,6 +1057,12 @@
 Builder.defineMacro("__CLANG_CUDA_APPROX_TRANSCENDENTALS__");
   }
 
+  // Define indicating that the source file is being compiled with a SYCL
+  // device compiler which doesn't produce host binary.
+  if (LangOpts.SYCLIsDevice) {
+Builder.defineMacro("__SYCL_DEVICE_ONLY__", "1");
+  }
+
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
 #define OPENCLEXT(Ext) 
\
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2879,6 +2879,8 @@
   << Opts.OMPHostIRFile;
   }
 
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+
   // Set CUDA mode for OpenMP target NVPTX if specified in options
   Opts.OpenMPCUDAMode = Opts.OpenMPIsDevice && T.isNVPTX() &&
 Args.hasArg(options::OPT_fopenmp_cuda_mode);
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -836,8 +836,14 @@
 def fopenmp_host_ir_file_path : Separate<["-"], "fopenmp-host-ir-file-path">,
   HelpText<"Path to the IR file produced by the frontend for the host.">;
 
-} // let Flags = [CC1Option]
+//===--===//
+// SYCL Options
+//===--===//
 
+def fsycl_is_device : Flag<["-"], "fsycl-is-device">,
+  HelpText<"Generate code for SYCL device.">;
+
+} // let Flags = [CC1Option]
 
 
//===--===//
 // cc1as-only Options
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -217,6 +217,8 @@
 LANGOPT(CUDADeviceApproxTranscendentals, 1, 0, "using approximate 
transcendental functions")
 LANGOPT(GPURelocatableDeviceCode, 1, 0, "generate relocatable device code")
 
+LANGOPT(SYCLIsDevice  , 1, 0, "Generate code for SYCL device")
+
 LANGOPT(SizedDeallocation , 1, 0, "sized deallocation")
 LANGOPT(AlignedAllocation , 1, 0, "aligned allocation")
 LANGOPT(AlignedAllocationUnavailable, 1, 0, "aligned allocation functions are 
unavailable")


Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 %s -E -dM | FileCheck %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+
+// CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1057,6 +1057,12 @@
 Builder.defineMacro("__CLANG_CUDA_APPROX_TRANSCENDENTALS__");
   }
 
+  // Define indicating that the source file is being compiled with a SYCL
+  // device compiler which doesn't produce host binary.
+  if (LangOpts.SYCLIsDevice) {
+Builder.defineMacro("__SYCL_DEVICE_ONLY__", "1");
+  }
+
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
 #define OPENCLEXT(Ext) \
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2879,6 +2879,8 @@
   << Opts.OMPHostIRFile;
   }
 
+  Opts.SYCLIsDevice = A

[PATCH] D58495: [clangd] Only report explicitly typed symbols during code navigation

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added inline comments.



Comment at: unittests/clangd/XRefsTests.cpp:462
   Foo a = $1^str;
-  Foo b = Foo($2^str);
+  Foo b = F$9^oo($2^str);
   Foo c = $3^f();

nit: keep the number in sort.



Comment at: unittests/clangd/XRefsTests.cpp:482
+  EXPECT_THAT(locateSymbolAt(AST, T.point("9")),
+  // First one is calss definition, second is the constructor.
+  ElementsAre(Sym("Foo"), Sym("Foo")));

s/calss/class


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58495



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

[PATCH] D58504: [OpenCL][8.0.0 Release] Notes for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: docs/ReleaseNotes.rst:14
 
-This document contains the release notes for the Clang C/C++/Objective-C
+This document contains the release notes for the Clang C/C++/Objective-C/OpenCL
 frontend, part of the LLVM Compiler Infrastructure, release 8.0.0. Here we

@hans  I am not sure if it's best to list all languages supported (i.e. OpenMP, 
Renderscript, CUDA are still missing) or alternatively we could change to 
something like - Clang C language family frontend? The latter one is taken from 
the main page: https://clang.llvm.org/. Although, OpenMP is missing on the main 
page.


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

https://reviews.llvm.org/D58504



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

[PATCH] D58504: [OpenCL][8.0.0 Release] Notes for OpenCL

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: docs/ReleaseNotes.rst:14
 
-This document contains the release notes for the Clang C/C++/Objective-C
+This document contains the release notes for the Clang C/C++/Objective-C/OpenCL
 frontend, part of the LLVM Compiler Infrastructure, release 8.0.0. Here we

Anastasia wrote:
> Anastasia wrote:
> > @hans  I am not sure if it's best to list all languages supported (i.e. 
> > OpenMP, Renderscript, CUDA are still missing) or alternatively we could 
> > change to something like - Clang C language family frontend? The latter one 
> > is taken from the main page: https://clang.llvm.org/. Although, OpenMP is 
> > missing on the main page.
> @ABataev should the main page be updated to list OpenMP explicitly?
Well, generally speaking, OpenMP is not a programming language, it is an API, 
i.e. just an extension for C/C++/Fortran. Not sure that it must be in the list 
of the actual languages.


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

https://reviews.llvm.org/D58504



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

[PATCH] D58495: [clangd] Only report explicitly typed symbols during code navigation

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE354585: [clangd] Only report explicitly typed symbols 
during code navigation (authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58495?vs=187786&id=187788#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58495

Files:
  clangd/XRefs.cpp
  unittests/clangd/SymbolInfoTests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -406,6 +406,16 @@
 
 double y = va^r;
   )cpp",
+
+  R"cpp(// No implicit constructors
+class X {
+  X(X&& x) = default;
+};
+X [[makeX]]() {}
+void foo() {
+  auto x = m^akeX();
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
@@ -453,20 +463,25 @@
   Foo c = $3^f();
   $4^g($5^f());
   g($6^str);
+  Foo ab$7^c;
+  Foo ab$8^cd("asdf");
+  Foo foox = Fo$9^o("asdf");
 }
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
   // Ordered assertions are deliberate: we expect a predictable order.
-  EXPECT_THAT(locateSymbolAt(AST, T.point("1")),
-  ElementsAre(Sym("str"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("1")), ElementsAre(Sym("str")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("2")), ElementsAre(Sym("str")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("3")),
-  ElementsAre(Sym("f"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("3")), ElementsAre(Sym("f")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("5")),
-  ElementsAre(Sym("f"), Sym("Foo")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("6")),
-  ElementsAre(Sym("str"), Sym("Foo"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("5")), ElementsAre(Sym("f")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("6")), ElementsAre(Sym("str")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("7")), ElementsAre(Sym("abc")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("8")),
+  ElementsAre(Sym("Foo"), Sym("abcd")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("9")),
+  // First one is class definition, second is the constructor.
+  ElementsAre(Sym("Foo"), Sym("Foo")));
 }
 
 TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -180,12 +180,8 @@
 func_baz1(f^f);
   }
 )cpp",
-  {
-  CreateExpectedSymbolDetails(
-  "ff", "func_baz2", "c:TestTU.cpp@218@F@func_baz2#@ff"),
-  CreateExpectedSymbolDetails(
-  "bar", "bar::", "c:@S@bar@F@bar#&1$@S@foo#"),
-  }},
+  {CreateExpectedSymbolDetails(
+  "ff", "func_baz2", "c:TestTU.cpp@218@F@func_baz2#@ff")}},
   {
   R"cpp( // Type reference - declaration
   struct foo;
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -110,20 +110,10 @@
   const MacroInfo *Info;
 };
 
-struct DeclInfo {
-  const Decl *D;
-  // Indicates the declaration is referenced by an explicit AST node.
-  bool IsReferencedExplicitly = false;
-};
-
 /// Finds declarations locations that a given source location refers to.
 class DeclarationAndMacrosFinder : public index::IndexDataConsumer {
   std::vector MacroInfos;
-  // The value of the map indicates whether the declaration has been referenced
-  // explicitly in the code.
-  // True means the declaration is explicitly referenced at least once; false
-  // otherwise.
-  llvm::DenseMap Decls;
+  llvm::DenseSet Decls;
   const SourceLocation &SearchedLocation;
   const ASTContext *
   Preprocessor &PP;
@@ -133,22 +123,14 @@
  ASTContext &AST, Preprocessor &PP)
   : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
 
-  // Get all DeclInfo of the found declarations.
-  // The results are sorted by "IsReferencedExplicitly" and declaration
-  // location.
-  std::vector getFoundDecls() const {
-std::vector Result;
-for (auto It : Decls) {
-  Result.emplace_back();
-  Result.back().D = It.first;
-  Result.back().IsReferencedExplicitly = It.second;
-}
-
-// Sort results. Declarations being referenced explicitly come first.
-llvm::sort(Result, [](const DeclInfo &L, const DeclInfo &R) {
-  if (L.IsReferencedExplicitly != R.IsReferencedExplicitly)
-re

[PATCH] D58495: [clangd] Only report explicitly typed symbols during code navigation

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 187786.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58495

Files:
  clangd/XRefs.cpp
  unittests/clangd/SymbolInfoTests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -406,6 +406,16 @@
 
 double y = va^r;
   )cpp",
+
+  R"cpp(// No implicit constructors
+class X {
+  X(X&& x) = default;
+};
+X [[makeX]]() {}
+void foo() {
+  auto x = m^akeX();
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
@@ -453,20 +463,25 @@
   Foo c = $3^f();
   $4^g($5^f());
   g($6^str);
+  Foo ab$7^c;
+  Foo ab$8^cd("asdf");
+  Foo foox = Fo$9^o("asdf");
 }
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
   // Ordered assertions are deliberate: we expect a predictable order.
-  EXPECT_THAT(locateSymbolAt(AST, T.point("1")),
-  ElementsAre(Sym("str"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("1")), ElementsAre(Sym("str")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("2")), ElementsAre(Sym("str")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("3")),
-  ElementsAre(Sym("f"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("3")), ElementsAre(Sym("f")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("5")),
-  ElementsAre(Sym("f"), Sym("Foo")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("6")),
-  ElementsAre(Sym("str"), Sym("Foo"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("5")), ElementsAre(Sym("f")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("6")), ElementsAre(Sym("str")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("7")), ElementsAre(Sym("abc")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("8")),
+  ElementsAre(Sym("Foo"), Sym("abcd")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("9")),
+  // First one is class definition, second is the constructor.
+  ElementsAre(Sym("Foo"), Sym("Foo")));
 }
 
 TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -180,12 +180,8 @@
 func_baz1(f^f);
   }
 )cpp",
-  {
-  CreateExpectedSymbolDetails(
-  "ff", "func_baz2", "c:TestTU.cpp@218@F@func_baz2#@ff"),
-  CreateExpectedSymbolDetails(
-  "bar", "bar::", "c:@S@bar@F@bar#&1$@S@foo#"),
-  }},
+  {CreateExpectedSymbolDetails(
+  "ff", "func_baz2", "c:TestTU.cpp@218@F@func_baz2#@ff")}},
   {
   R"cpp( // Type reference - declaration
   struct foo;
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -110,20 +110,10 @@
   const MacroInfo *Info;
 };
 
-struct DeclInfo {
-  const Decl *D;
-  // Indicates the declaration is referenced by an explicit AST node.
-  bool IsReferencedExplicitly = false;
-};
-
 /// Finds declarations locations that a given source location refers to.
 class DeclarationAndMacrosFinder : public index::IndexDataConsumer {
   std::vector MacroInfos;
-  // The value of the map indicates whether the declaration has been referenced
-  // explicitly in the code.
-  // True means the declaration is explicitly referenced at least once; false
-  // otherwise.
-  llvm::DenseMap Decls;
+  llvm::DenseSet Decls;
   const SourceLocation &SearchedLocation;
   const ASTContext *
   Preprocessor &PP;
@@ -133,22 +123,14 @@
  ASTContext &AST, Preprocessor &PP)
   : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
 
-  // Get all DeclInfo of the found declarations.
-  // The results are sorted by "IsReferencedExplicitly" and declaration
-  // location.
-  std::vector getFoundDecls() const {
-std::vector Result;
-for (auto It : Decls) {
-  Result.emplace_back();
-  Result.back().D = It.first;
-  Result.back().IsReferencedExplicitly = It.second;
-}
+  // The results are sorted by declaration location.
+  std::vector getFoundDecls() const {
+std::vector Result;
+for (const Decl *D : Decls)
+  Result.push_back(D);
 
-// Sort results. Declarations being referenced explicitly come first.
-llvm::sort(Result, [](const DeclInfo &L, const DeclInfo &R) {
-  if (L.IsReferencedExplicitly != R.IsReferencedExp

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D57914#1405665 , @pgousseau wrote:

> In D57914#1405615 , @riccibruno 
> wrote:
>
> > I think what you would really want to do is mark the masks as `inline 
> > constexpr`, but sadly inline variables are C++17 only. I have seen some 
> > ways to emulate inline variables but I am not sure whether it is worth 
> > doing so in this case.
>
>
> Yes thanks for the advice.
>  I have tried moving the definitions to the cpp file but ran into 
> initialization order fiasco issue because of SanitizerArgs.cpp global 
> definitions.
>  If using a constexpr SanitizerMask ctor then that would work around the 
> initialization fiasco issue hopefully?
>  Although I am not having much luck using constexpr ctor in VS2015 because of 
> the array member, so might have to revert back to lo/hi mask members?


I would prefer to avoid doing this. What is the problem with `constexpr` ?


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

https://reviews.llvm.org/D57914



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

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:816
+// If the client supports Markdown, convert from plaintext here.
+if (*H && HoverSupportsMarkdown) {
+  (*H)->contents.kind = MarkupKind::Markdown;

malaperle wrote:
> I don't know if you meant plain text as non-language-specific markdown or no 
> markdown at all. In VSCode, non-markdown for multiline macros looks bad 
> because the newlines are ignored.
Did not know about that, thanks for pointing it out.
It does not ignore double newlines though (that's what we use in declarations). 
I suspect it treats plaintext as markdown, but can't be sure.

In any case, converting to a markdown code block here looks incredibly hacky 
and shaky.
Could we use double-newlines for line breaks in our implementation instead?

This aligns with what we did before this patch for declarations.
If that doesn't work, breaking the multi-line macros in VSCode looks ok, this 
really feels like a bug in VSCode.



Comment at: clangd/Protocol.cpp:251
+auto *A = HoverMarkupKinds->getAsArray();
+if (!A) {
+  return false;

NIT: remove braces 



Comment at: clangd/Protocol.cpp:255
+
+for (size_t I = 0; I < A->size(); ++I) {
+  MarkupKind KindOut;

NIT: use range-based-for:
```
for (auto &&E : A) {
 // ...
}
```



Comment at: clangd/Protocol.cpp:257
+  MarkupKind KindOut;
+  if (fromJSON((*A)[I], KindOut)) {
+if (KindOut == MarkupKind::Markdown) {

NIT: merge ifs for better readability
``` 
if (fromJSON(...) && KindOut == Markdown) {
  // 
}
```



Comment at: clangd/Protocol.cpp:638
+bool fromJSON(const llvm::json::Value &E, MarkupKind &Out) {
+  if (auto T = E.getAsString()) {
+if (*T == "plaintext")

use [[ 
https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code
 | early exits ]] to reduce nesting.

```
auto T = E.getAsString();
if (!T)
  return false;
// rest of the code...
```




Comment at: clangd/XRefs.cpp:563
+  H.contents.kind = MarkupKind::PlainText;
+  H.contents.value = "#define " + Definition;
   return H;

Follow-up for the previous comment:
would it work to do `replace(Defintion, "\n", "\n\n")` here? (with a fixme 
comment that otherwise VSCode misbehaves)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55250



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

[PATCH] D58504: [OpenCL][8.0.0 Release] Notes for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: docs/ReleaseNotes.rst:14
 
-This document contains the release notes for the Clang C/C++/Objective-C
+This document contains the release notes for the Clang C/C++/Objective-C/OpenCL
 frontend, part of the LLVM Compiler Infrastructure, release 8.0.0. Here we

ABataev wrote:
> Anastasia wrote:
> > Anastasia wrote:
> > > @hans  I am not sure if it's best to list all languages supported (i.e. 
> > > OpenMP, Renderscript, CUDA are still missing) or alternatively we could 
> > > change to something like - Clang C language family frontend? The latter 
> > > one is taken from the main page: https://clang.llvm.org/. Although, 
> > > OpenMP is missing on the main page.
> > @ABataev should the main page be updated to list OpenMP explicitly?
> Well, generally speaking, OpenMP is not a programming language, it is an API, 
> i.e. just an extension for C/C++/Fortran. Not sure that it must be in the 
> list of the actual languages.
Ok, then I guess it will be ok to leave it out in this update to the release 
notes too!


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

https://reviews.llvm.org/D58504



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

[PATCH] D58509: [CodeGen] Fix string literal address space casting.

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
hliao added a reviewer: yaxunl.
Herald added subscribers: cfe-commits, jvesely.
Herald added a project: clang.

- If a string literal is reused directly, need to add necessary address space 
casting if the target requires that.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58509

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/amdgcn-string-literal.cpp


Index: clang/test/CodeGenCXX/amdgcn-string-literal.cpp
===
--- clang/test/CodeGenCXX/amdgcn-string-literal.cpp
+++ clang/test/CodeGenCXX/amdgcn-string-literal.cpp
@@ -14,7 +14,7 @@
 // CHECK-LABEL: define void @_Z1fv()
 void f() {
   const char* l_str = "l_str";
-  
+
   // CHECK: call void @llvm.memcpy.p0i8.p4i8.i64
   char l_array[] = "l_array";
 
@@ -26,3 +26,9 @@
   const char* p = g_str;
   g(p);
 }
+
+// CHECK-LABEL: define void @_Z1ev
+void e() {
+  g("string literal");
+  g("string literal");
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4522,7 +4522,8 @@
 if (auto GV = *Entry) {
   if (Alignment.getQuantity() > GV->getAlignment())
 GV->setAlignment(Alignment.getQuantity());
-  return ConstantAddress(GV, Alignment);
+  return ConstantAddress(castStringLiteralToDefaultAddressSpace(*this, GV),
+ Alignment);
 }
   }
 
@@ -4584,7 +4585,8 @@
 if (auto GV = *Entry) {
   if (Alignment.getQuantity() > GV->getAlignment())
 GV->setAlignment(Alignment.getQuantity());
-  return ConstantAddress(GV, Alignment);
+  return ConstantAddress(castStringLiteralToDefaultAddressSpace(*this, GV),
+ Alignment);
 }
   }
 


Index: clang/test/CodeGenCXX/amdgcn-string-literal.cpp
===
--- clang/test/CodeGenCXX/amdgcn-string-literal.cpp
+++ clang/test/CodeGenCXX/amdgcn-string-literal.cpp
@@ -14,7 +14,7 @@
 // CHECK-LABEL: define void @_Z1fv()
 void f() {
   const char* l_str = "l_str";
-  
+
   // CHECK: call void @llvm.memcpy.p0i8.p4i8.i64
   char l_array[] = "l_array";
 
@@ -26,3 +26,9 @@
   const char* p = g_str;
   g(p);
 }
+
+// CHECK-LABEL: define void @_Z1ev
+void e() {
+  g("string literal");
+  g("string literal");
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4522,7 +4522,8 @@
 if (auto GV = *Entry) {
   if (Alignment.getQuantity() > GV->getAlignment())
 GV->setAlignment(Alignment.getQuantity());
-  return ConstantAddress(GV, Alignment);
+  return ConstantAddress(castStringLiteralToDefaultAddressSpace(*this, GV),
+ Alignment);
 }
   }
 
@@ -4584,7 +4585,8 @@
 if (auto GV = *Entry) {
   if (Alignment.getQuantity() > GV->getAlignment())
 GV->setAlignment(Alignment.getQuantity());
-  return ConstantAddress(GV, Alignment);
+  return ConstantAddress(castStringLiteralToDefaultAddressSpace(*this, GV),
+ Alignment);
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58504: [OpenCL][8.0.0 Release] Notes for OpenCL

[Alexey Sotkin via Phabricator via cfe-commits]

AlexeySotkin added inline comments.



Comment at: docs/ReleaseNotes.rst:228
 
-OpenCL C Language Changes in Clang
+OpenCL Language Changes in Clang
 --

Why the "C" is removed ?


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

https://reviews.llvm.org/D58504



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

r354593 - [CUDA]Delayed diagnostics for the asm instructions.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Feb 21 07:51:30 2019
New Revision: 354593

URL: http://llvm.org/viewvc/llvm-project?rev=354593&view=rev
Log:
[CUDA]Delayed diagnostics for the asm instructions.

Summary:
Adapted targetDiag for the CUDA and used for the delayed diagnostics in
asm constructs. Works for both host and device compilation sides.

Reviewers: tra, jlebar

Subscribers: jdoerfert, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu
Modified:
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Sema/SemaStmtAsm.cpp

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=354593&r1=354592&r2=354593&view=diff
==
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Thu Feb 21 07:51:30 2019
@@ -1487,10 +1487,12 @@ void Sema::markKnownEmitted(
   }
 }
 
-Sema::DeviceDiagBuilder Sema::targetDiag(SourceLocation Loc,
- unsigned DiagID) {
+Sema::DeviceDiagBuilder Sema::targetDiag(SourceLocation Loc, unsigned DiagID) {
   if (LangOpts.OpenMP && LangOpts.OpenMPIsDevice)
 return diagIfOpenMPDeviceCode(Loc, DiagID);
+  if (getLangOpts().CUDA)
+return getLangOpts().CUDAIsDevice ? CUDADiagIfDeviceCode(Loc, DiagID)
+  : CUDADiagIfHostCode(Loc, DiagID);
   return DeviceDiagBuilder(DeviceDiagBuilder::K_Immediate, Loc, DiagID,
getCurFunctionDecl(), *this);
 }

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=354593&r1=354592&r2=354593&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Thu Feb 21 07:51:30 2019
@@ -750,7 +750,7 @@ ExprResult Sema::BuildCXXThrow(SourceLoc
bool IsThrownVarInScope) {
   // Don't report an error if 'throw' is used in system headers.
   if (!getLangOpts().CXXExceptions &&
-  !getSourceManager().isInSystemHeader(OpLoc)) {
+  !getSourceManager().isInSystemHeader(OpLoc) && !getLangOpts().CUDA) {
 // Delay error emission for the OpenMP device code.
 targetDiag(OpLoc, diag::err_exceptions_disabled) << "throw";
   }

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=354593&r1=354592&r2=354593&view=diff
==
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Feb 21 07:51:30 2019
@@ -3993,7 +3993,7 @@ StmtResult Sema::ActOnCXXTryBlock(Source
   ArrayRef Handlers) {
   // Don't report an error if 'try' is used in system headers.
   if (!getLangOpts().CXXExceptions &&
-  !getSourceManager().isInSystemHeader(TryLoc)) {
+  !getSourceManager().isInSystemHeader(TryLoc) && !getLangOpts().CUDA) {
 // Delay error emission for the OpenMP device code.
 targetDiag(TryLoc, diag::err_exceptions_disabled) << "try";
   }

Modified: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAsm.cpp?rev=354593&r1=354592&r2=354593&view=diff
==
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp Thu Feb 21 07:51:30 2019
@@ -253,15 +253,6 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceL
   // The parser verifies that there is a string literal here.
   assert(AsmString->isAscii());
 
-  // If we're compiling CUDA file and function attributes indicate that it's 
not
-  // for this compilation side, skip all the checks.
-  if (!DeclAttrsMatchCUDAMode(getLangOpts(), getCurFunctionDecl())) {
-GCCAsmStmt *NS = new (Context) GCCAsmStmt(
-Context, AsmLoc, IsSimple, IsVolatile, NumOutputs, NumInputs, Names,
-Constraints, Exprs.data(), AsmString, NumClobbers, Clobbers, 
RParenLoc);
-return NS;
-  }
-
   for (unsigned i = 0; i != NumOutputs; i++) {
 StringLiteral *Literal = Constraints[i];
 assert(Literal->isAscii());

Added: cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu?rev=354593&view=auto
==
--- cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu (added)
+++ cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu Thu Feb 21 07:51:30 2019
@@ -0,0 +1,118 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DHOST -triple 
x86_64-unknown-linux-gnu
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DHOST -DHOST_USED -triple 
x86_64-unknown

[PATCH] D58463: [CUDA]Delayed diagnostics for the asm instructions.

[Alexey Bataev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354593: [CUDA]Delayed diagnostics for the asm instructions. 
(authored by ABataev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58463?vs=187621&id=187798#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58463

Files:
  cfe/trunk/lib/Sema/Sema.cpp
  cfe/trunk/lib/Sema/SemaExprCXX.cpp
  cfe/trunk/lib/Sema/SemaStmt.cpp
  cfe/trunk/lib/Sema/SemaStmtAsm.cpp
  cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu

Index: cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu
===
--- cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu
+++ cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu
@@ -0,0 +1,118 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DHOST -triple x86_64-unknown-linux-gnu
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DHOST -DHOST_USED -triple x86_64-unknown-linux-gnu
+// RUN: %clang_cc1 -fsyntax-only -fcuda-is-device -verify %s -DDEVICE_NOT_USED -triple nvptx-unknown-cuda
+// RUN: %clang_cc1 -fsyntax-only -fcuda-is-device -verify %s -DDEVICE -triple nvptx-unknown-cuda
+// RUN: %clang_cc1 -fsyntax-only -fcuda-is-device -verify %s -DDEVICE -DDEVICE_USED -triple nvptx-unknown-cuda
+
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+#if (defined(HOST) && !defined(HOST_USED)) || defined(DEVICE_NOT_USED)
+// expected-no-diagnostics
+#endif
+
+#include "Inputs/cuda.h"
+
+static __device__ __host__ void t1(int r) {
+  __asm__("PR3908 %[lf] %[xx] %[li] %[r]"
+  : [ r ] "+r"(r)
+  : [ lf ] "mx"(0), [ li ] "mr"(0), [ xx ] "x"((double)(0)));
+}
+
+static __device__ __host__ unsigned t2(signed char input) {
+  unsigned output;
+  __asm__("xyz"
+  : "=a"(output)
+  : "0"(input));
+  return output;
+}
+
+static __device__ __host__ double t3(double x) {
+  register long double result;
+  __asm __volatile("frndint"
+   : "=t"(result)
+   : "0"(x));
+  return result;
+}
+
+static __device__ __host__ unsigned char t4(unsigned char a, unsigned char b) {
+  unsigned int la = a;
+  unsigned int lb = b;
+  unsigned int bigres;
+  unsigned char res;
+  __asm__("0:\n1:\n"
+  : [ bigres ] "=la"(bigres)
+  : [ la ] "0"(la), [ lb ] "c"(lb)
+  : "edx", "cc");
+  res = bigres;
+  return res;
+}
+
+static __device__ __host__ void t5(void) {
+  __asm__ __volatile__(
+  "finit"
+  :
+  :
+  : "st", "st(1)", "st(2)", "st(3)",
+"st(4)", "st(5)", "st(6)", "st(7)",
+"fpsr", "fpcr");
+}
+
+typedef long long __m256i __attribute__((__vector_size__(32)));
+static __device__ __host__ void t6(__m256i *p) {
+  __asm__ volatile("vmovaps  %0, %%ymm0" ::"m"(*(__m256i *)p)
+   : "ymm0");
+}
+
+static __device__ __host__ void t7(__m256i *p) {
+  __asm__ volatile("vmovaps  %0, %%ymm0" ::"m"(*(__m256i *)p)
+   : "r0");
+}
+
+#ifdef DEVICE
+__device__ int m() {
+  t1(0);
+  t2(0);
+  t3(0);
+  t4(0, 0);
+  t5();
+  t6(0);
+#ifdef DEVICE_USED
+  t7(0);
+#endif // DEVICE_USED
+  return 0;
+}
+#endif // DEVICE
+
+#ifdef HOST
+__host__ int main() {
+  t1(0);
+  t2(0);
+  t3(0);
+  t4(0, 0);
+  t5();
+  t6(0);
+#ifdef HOST_USED
+  t7(0);
+#endif // HOST_USED
+  return 0;
+}
+#endif // HOST
+
+#if defined(HOST_USED)
+// expected-error@69 {{unknown register name 'r0' in asm}}
+// expected-note@96 {{called by 'main'}}
+#elif defined(DEVICE)
+// expected-error@19 {{invalid input constraint 'mx' in asm}}
+// expected-error@25 {{invalid output constraint '=a' in asm}}
+// expected-error@33 {{invalid output constraint '=t' in asm}}
+// expected-error@44 {{invalid output constraint '=la' in asm}}
+// expected-error@56 {{unknown register name 'st' in asm}}
+// expected-error@64 {{unknown register name 'ymm0' in asm}}
+// expected-note@74 {{called by 'm'}}
+// expected-note@75 {{called by 'm'}}
+// expected-note@76 {{called by 'm'}}
+// expected-note@77 {{called by 'm'}}
+// expected-note@78 {{called by 'm'}}
+// expected-note@79 {{called by 'm'}}
+#endif
Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp
@@ -750,7 +750,7 @@
bool IsThrownVarInScope) {
   // Don't report an error if 'throw' is used in system headers.
   if (!getLangOpts().CXXExceptions &&
-  !getSourceManager().isInSystemHeader(OpLoc)) {
+  !getSourceManager().isInSystemHeader(OpLoc) && !getLangOpts().CUDA) {
 // Delay error emission for the OpenMP device code.
 targetDiag(OpLoc, diag::err_exceptions_disabled) << "throw";
   }
Index: cfe/trunk/lib/Sema/Sema.cpp
===
--- cfe/trunk/lib/Sema/S

[PATCH] D58504: [OpenCL][8.0.0 Release] Notes for OpenCL

[Alexey Sotkin via Phabricator via cfe-commits]

AlexeySotkin added inline comments.



Comment at: docs/ReleaseNotes.rst:228
 
-OpenCL C Language Changes in Clang
+OpenCL Language Changes in Clang
 --

AlexeySotkin wrote:
> Why the "C" is removed ?
Should we call the section like: "OpenCL Support in Clang"?


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

https://reviews.llvm.org/D58504



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

[PATCH] D58504: [OpenCL][8.0.0 Release] Notes for OpenCL

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 added inline comments.



Comment at: docs/ReleaseNotes.rst:14
 
-This document contains the release notes for the Clang C/C++/Objective-C
+This document contains the release notes for the Clang C/C++/Objective-C/OpenCL
 frontend, part of the LLVM Compiler Infrastructure, release 8.0.0. Here we

Anastasia wrote:
> ABataev wrote:
> > Anastasia wrote:
> > > Anastasia wrote:
> > > > @hans  I am not sure if it's best to list all languages supported (i.e. 
> > > > OpenMP, Renderscript, CUDA are still missing) or alternatively we could 
> > > > change to something like - Clang C language family frontend? The latter 
> > > > one is taken from the main page: https://clang.llvm.org/. Although, 
> > > > OpenMP is missing on the main page.
> > > @ABataev should the main page be updated to list OpenMP explicitly?
> > Well, generally speaking, OpenMP is not a programming language, it is an 
> > API, i.e. just an extension for C/C++/Fortran. Not sure that it must be in 
> > the list of the actual languages.
> Ok, then I guess it will be ok to leave it out in this update to the release 
> notes too!
I agree with Alexey.  OpenMP is an API specification not quite a language yet.


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

https://reviews.llvm.org/D58504



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

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[pierre gousseau via Phabricator via cfe-commits]

pgousseau added a comment.

In D57914#1405781 , @riccibruno wrote:

> In D57914#1405665 , @pgousseau wrote:
>
> > In D57914#1405615 , @riccibruno 
> > wrote:
> >
> > > I think what you would really want to do is mark the masks as `inline 
> > > constexpr`, but sadly inline variables are C++17 only. I have seen some 
> > > ways to emulate inline variables but I am not sure whether it is worth 
> > > doing so in this case.
> >
> >
> > Yes thanks for the advice.
> >  I have tried moving the definitions to the cpp file but ran into 
> > initialization order fiasco issue because of SanitizerArgs.cpp global 
> > definitions.
> >  If using a constexpr SanitizerMask ctor then that would work around the 
> > initialization fiasco issue hopefully?
> >  Although I am not having much luck using constexpr ctor in VS2015 because 
> > of the array member, so might have to revert back to lo/hi mask members?
>
>
> I would prefer to avoid doing this. What is the problem with `constexpr` ?


I thought of using `constexpr SanitizerMask Foo = {lo, hi};` as this seems 
supported in VS2015 but I cant seem to find a way to get this to work with 
array members...
Now I also realised I would need something like `constexpr SanitizerMask 
FooGroup = Foo + Bar;`
And I havent found a way yet, arrays or no arrays, any ideas?


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

https://reviews.llvm.org/D57914



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

r354596 - Revert "[CUDA]Delayed diagnostics for the asm instructions."

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Feb 21 08:40:21 2019
New Revision: 354596

URL: http://llvm.org/viewvc/llvm-project?rev=354596&view=rev
Log:
Revert "[CUDA]Delayed diagnostics for the asm instructions."

This reverts commit r354593 to fix the problem with the crash on
windows.

Removed:
cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu
Modified:
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Sema/SemaStmtAsm.cpp

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=354596&r1=354595&r2=354596&view=diff
==
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Thu Feb 21 08:40:21 2019
@@ -1487,12 +1487,10 @@ void Sema::markKnownEmitted(
   }
 }
 
-Sema::DeviceDiagBuilder Sema::targetDiag(SourceLocation Loc, unsigned DiagID) {
+Sema::DeviceDiagBuilder Sema::targetDiag(SourceLocation Loc,
+ unsigned DiagID) {
   if (LangOpts.OpenMP && LangOpts.OpenMPIsDevice)
 return diagIfOpenMPDeviceCode(Loc, DiagID);
-  if (getLangOpts().CUDA)
-return getLangOpts().CUDAIsDevice ? CUDADiagIfDeviceCode(Loc, DiagID)
-  : CUDADiagIfHostCode(Loc, DiagID);
   return DeviceDiagBuilder(DeviceDiagBuilder::K_Immediate, Loc, DiagID,
getCurFunctionDecl(), *this);
 }

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=354596&r1=354595&r2=354596&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Thu Feb 21 08:40:21 2019
@@ -750,7 +750,7 @@ ExprResult Sema::BuildCXXThrow(SourceLoc
bool IsThrownVarInScope) {
   // Don't report an error if 'throw' is used in system headers.
   if (!getLangOpts().CXXExceptions &&
-  !getSourceManager().isInSystemHeader(OpLoc) && !getLangOpts().CUDA) {
+  !getSourceManager().isInSystemHeader(OpLoc)) {
 // Delay error emission for the OpenMP device code.
 targetDiag(OpLoc, diag::err_exceptions_disabled) << "throw";
   }

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=354596&r1=354595&r2=354596&view=diff
==
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Feb 21 08:40:21 2019
@@ -3993,7 +3993,7 @@ StmtResult Sema::ActOnCXXTryBlock(Source
   ArrayRef Handlers) {
   // Don't report an error if 'try' is used in system headers.
   if (!getLangOpts().CXXExceptions &&
-  !getSourceManager().isInSystemHeader(TryLoc) && !getLangOpts().CUDA) {
+  !getSourceManager().isInSystemHeader(TryLoc)) {
 // Delay error emission for the OpenMP device code.
 targetDiag(TryLoc, diag::err_exceptions_disabled) << "try";
   }

Modified: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAsm.cpp?rev=354596&r1=354595&r2=354596&view=diff
==
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp Thu Feb 21 08:40:21 2019
@@ -253,6 +253,15 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceL
   // The parser verifies that there is a string literal here.
   assert(AsmString->isAscii());
 
+  // If we're compiling CUDA file and function attributes indicate that it's 
not
+  // for this compilation side, skip all the checks.
+  if (!DeclAttrsMatchCUDAMode(getLangOpts(), getCurFunctionDecl())) {
+GCCAsmStmt *NS = new (Context) GCCAsmStmt(
+Context, AsmLoc, IsSimple, IsVolatile, NumOutputs, NumInputs, Names,
+Constraints, Exprs.data(), AsmString, NumClobbers, Clobbers, 
RParenLoc);
+return NS;
+  }
+
   for (unsigned i = 0; i != NumOutputs; i++) {
 StringLiteral *Literal = Constraints[i];
 assert(Literal->isAscii());

Removed: cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu?rev=354595&view=auto
==
--- cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu (original)
+++ cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu (removed)
@@ -1,118 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -DHOST -triple 
x86_64-unknown-linux-gnu
-// RUN: %clang_cc1 -fsyntax-only -verify %s -DHOST -DHOST_USED -triple 
x86_64-unknown-linux-gnu
-// RUN: %clang_cc1 -fsyntax-only -fcuda-is-device -verify %s -DDEVICE_NOT_USED 
-triple nvptx-unknown-cuda
-// RUN: %clang_cc1 -fsyntax-only -fcuda-is-device -verify %s -DDEVICE -triple 
nvpt

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: rjmccall, erik.pilkington.
ahatanak added a project: clang.
Herald added subscribers: jdoerfert, dexonsmith, jkorous.

This patch avoids copying blocks that initialize or are assigned to local auto 
variables to the heap when the local auto variables do not have their addresses 
taken and are declared in the same scope as the block. We can possibly add back 
the optimization in the ARC optimizer that was reverted in r189869 
(http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20130902/186509.html),
 but I suspect it would be much more complicated and fragile than doing it in 
the front-end.

I'm not 100% sure whether it's necessary to disable this optimization when the 
address of the local variable is taken. We do pass a block on the stack to a 
function when the block is directly passed instead of first being assigned to a 
local variable, but clang currently doesn't copy the passed block to the heap 
in the callee although the block can possibly escape if the address is taken. 
For example:

  __strong id *g0, g1;
  
  void foo0(BlockTy b) {
g0 = (__strong id *)&b;
g1 = *g0; // this is just a retain, not a block copy.
  }
  
  void foo1() {
foo0(^{...}) // block is on the stack.
  }
  
  void foo2() {
foo1();
((BlockTy)g1)(); // this can crash if the block is still on the stack.
  }

rdar://problem/13289333


Repository:
  rC Clang

https://reviews.llvm.org/D58514

Files:
  include/clang/AST/Decl.h
  include/clang/AST/DeclBase.h
  include/clang/Sema/ScopeInfo.h
  lib/AST/Decl.cpp
  lib/CodeGen/CGObjC.cpp
  lib/Sema/ScopeInfo.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenObjC/arc-block-copy-escape.m
  test/CodeGenObjC/arc-blocks.m
  test/CodeGenObjCXX/arc-blocks.mm
  test/PCH/arc-blocks.mm

Index: test/PCH/arc-blocks.mm
===
--- /dev/null
+++ test/PCH/arc-blocks.mm
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -fobjc-arc -fblocks -std=c++1y -emit-pch %s -o %t
+// RUN: %clang_cc1 -fobjc-arc -fblocks -std=c++1y -include-pch %t -emit-llvm -o - %s | FileCheck %s
+
+#ifndef HEADER_INCLUDED
+#define HEADER_INCLUDED
+
+namespace test_block_retain {
+  typedef void (^BlockTy)();
+  void foo1(id);
+
+  inline void initialization(id a) {
+// Call to @llvm.objc.retainBlock isn't needed.
+BlockTy b0 = ^{ foo1(a); };
+b0();
+  }
+
+  inline void assignmentConditional(id a, bool c) {
+BlockTy b0;
+if (c)
+  // @llvm.objc.retainBlock is called since 'b0' is declared in the outer scope.
+  b0 = ^{ foo1(a); };
+b0();
+  }
+}
+
+#else
+
+// CHECK: %[[STRUCT_BLOCK_DESCRIPTOR:.*]] = type { i64, i64 }
+
+namespace test_block_retain {
+// CHECK-LABEL: define linkonce_odr void @_ZN17test_block_retain14initializationEP11objc_object(
+// CHECK-NOT: call i8* @llvm.objc.retainBlock(
+
+  void test_initialization(id a) {
+initialization(a);
+  }
+
+// CHECK-LABEL: define void @_ZN17test_block_retain26test_assignmentConditionalEP11objc_objectb(
+// CHECK: %[[BLOCK:.*]] = alloca <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8* }>, align 8
+// CHECK: %[[V4:.*]] = bitcast <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8* }>* %[[BLOCK]] to void ()*
+// CHECK: %[[V5:.*]] = bitcast void ()* %[[V4]] to i8*
+// CHECK: call i8* @llvm.objc.retainBlock(i8* %[[V5]])
+
+  void test_assignmentConditional(id a, bool c) {
+assignmentConditional(a, c);
+  }
+}
+
+#endif
Index: test/CodeGenObjCXX/arc-blocks.mm
===
--- test/CodeGenObjCXX/arc-blocks.mm
+++ test/CodeGenObjCXX/arc-blocks.mm
@@ -201,3 +201,141 @@
   ^{ (void)t0; (void)t1; (void)t2; (void)t3; (void)t4; (void)t5; };
 }
 }
+
+// Test that calls to @llvm.objc.retainBlock aren't emitted in some cases.
+
+namespace test_block_retain {
+  typedef void (^BlockTy)();
+
+  void foo1(id);
+
+// CHECK-LABEL: define void @_ZN17test_block_retain14initializationEP11objc_object(
+// CHECK-NOT: @llvm.objc.retainBlock(
+  void initialization(id a) {
+BlockTy b0 = ^{ foo1(a); };
+b0();
+  }
+
+// CHECK-LABEL: define void @_ZN17test_block_retain20initializationStaticEP11objc_object(
+// CHECK: @llvm.objc.retainBlock(
+  void initializationStatic(id a) {
+static BlockTy b0 = ^{ foo1(a); };
+b0();
+  }
+
+// CHECK-LABEL: define void @_ZN17test_block_retain15initialization2EP11objc_object
+// CHECK: %[[B0:.*]] = alloca void ()*, align 8
+// CHECK: %[[B1:.*]] = alloca void ()*, align 8
+// CHECK: load void ()*, void ()** %[[B0]], align 8
+// CHECK-NOT: @llvm.objc.retainBlock
+// CHECK: %[[V9:.*]] = load void ()*, void ()** %[[B0]], align 8
+// CHECK: %[[V10:.*]] = bitcast void ()* %[[V9]] to i8*
+// CHECK: %[[V11:.*]] = call i8* @llvm.objc.retainBlock(i8* %[[V10]])
+// CHECK: %[[V12:.*]] = bitcast i8* %[[V11]] to void ()*
+// CH

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

To define the masks as `constexpr` variables you will have to make at least 
`bitPosToMask`, `operator|`, `operator&` and `operator~`. Unfortunately 
`constexpr` functions in c++11 are rather restricted. It seems to me however 
that you could do the following:

1. Wait until we can use C++14 in llvm (soon hopefully) to mark the masks 
`contexpr` (and leave a FIXME for now).
2. Solve the odr-violation issue by using one of the work-around in n4424. In 
particular I like the second work-around consisting of putting the static 
members in a class template.


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

https://reviews.llvm.org/D57914



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

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

More explicitly something like:

in `Sanitizer.h`:

  template  struct SanitizerMasks {
static const SanitizerMask SomeMask;
/* and so on for each mask*/
  };
  
  template  const SanitizerMask SanitizerMasks::SomeMask = the 
definition;

And then you can write `SanitizerMasks<>::SomeMask` when you want to use this 
mask.


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

https://reviews.llvm.org/D57914



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I'm not a code owner here but we seem to be lacking people who are either 
available, able, willing or allowed to approve code reviews for some of the 
code in the clang-tooling (not just clang format).  The code owners do an 
extremely good job at doing a deep review way beyond what many of the rest of 
us are able to do..  They've been doing this for years having written may of 
these tools in the first place, but I suspect other external pressures and 
their phabricator activity suggests they have limited time to contribute at 
present.

As such code reviews for new features seem to sit unreviewed  and unaccepted 
and yet the subscribers list and tokens, the pings and "whats blocking this" 
comments would suggest that revisions such as these are wanted but sit for 
weeks and months with no movement.

It feels like we need more people reviewing code and approving code changes 
otherwise these revisions bit rot or simply do not get into the tool. It 
definitely will disenfranchise new developers from contributing, if it takes 
months/years to get anything committed. But a shallow review by someone with at 
least a little experience of this code is surely better than no review at all. 
As such I'd like to volunteer to help review some of these items.

Not all areas of clang have this same problem, I see areas of code which 
sometimes don't even get a review, where buddies review each others code very 
quickly, or where the time from creation to commit can be measured in hours or 
days, not weeks or months. I'm unclear what makes commits here take so long and 
good improvements lost. As such I don't think we get people contributing to 
fixing bugs either because the time taken to get even a bug landed seems quite 
great.

From what I understand as long as changes meet the guidelines set out about 
adding new options, changes have tests and have proper documentation then we 
should be free to approve and land those changes, The code owners have been 
given time like the rest of us to review and in the event they don't like the 
commit after the fact are free to submit a revision later or ask for further 
modifications. (make sure you check the done message on each of their review 
comments as you address them and ensure you leave a comment if you are not 
going to do the suggested change and why)

As developers if we submit a revision we need to be prepared to look after that 
change and address and defects that come in, we don't want people driving by 
and dropping a patch that the code owners have to maintain, so with committing 
come responsibility.

I've personally been running with this patch in my clang format since 
@VelocityRa  took it over, and from what I can tell it works well, I too would 
like to see this landed.

With that said @VelocityRa and this comment, I would ask that you add something 
to the docs/ReleaseNotes.rst to show the addition of the new option to 
clang-format, and if you make that change, rebase (I doubt anything else was 
accepted in the meantime!) and send an updated revision I will add a LGTM and 
accept the review (for what that is worth), this will give @djasper  and 
@klimek  time to object to my proposal here.

Assuming others are in agreement, lets do something to move at least this 
forward.


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

https://reviews.llvm.org/D28462



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D28462#1405711 , @klimek wrote:

> In D28462#1405360 , @MyDeveloperDay 
> wrote:
>
> > I'm not a code owner here but we seem to be lacking people who are either 
> > available, able, willing or allowed to approve code reviews for some of the 
> > code in the clang-tooling (not just clang format).  The code owners do an 
> > extremely good job at doing a deep review way beyond what many of the rest 
> > of us are able to do..  They've been doing this for years having written 
> > may of these tools in the first place, but I suspect other external 
> > pressures and their phabricator activity suggests they have limited time to 
> > contribute at present.
> >
> > As such code reviews for new features seem to sit unreviewed  and 
> > unaccepted and yet the subscribers list and tokens, the pings and "whats 
> > blocking this" comments would suggest that revisions such as these are 
> > wanted but sit for weeks and months with no movement.
> >
> > It feels like we need more people reviewing code and approving code changes 
> > otherwise these revisions bit rot or simply do not get into the tool. It 
> > definitely will disenfranchise new developers from contributing, if it 
> > takes months/years to get anything committed. But a shallow review by 
> > someone with at least a little experience of this code is surely better 
> > than no review at all. As such I'd like to volunteer to help review some of 
> > these items.
>
>
> I don't think shallow reviews before having the architectural questions tied 
> down would help a lot, as they in my experience create more opposition of 
> change later on.
>
> I fully agree that the state we're in is bad, and I'm truly sorry for us to 
> have arrived here.
>
> > Not all areas of clang have this same problem, I see areas of code which 
> > sometimes don't even get a review, where buddies review each others code 
> > very quickly, or where the time from creation to commit can be measured in 
> > hours or days, not weeks or months. I'm unclear what makes commits here 
> > take so long and good improvements lost. As such I don't think we get 
> > people contributing to fixing bugs either because the time taken to get 
> > even a bug landed seems quite great.
>
> One of the differences is whether there is ahead-of-code-review-time 
> agreement on the direction, and whether the code fits in well from an 
> architectural point of view.
>  In clang-format we specifically do not want to support all possible 
> features, but carefully weigh the upside of features against maintenance 
> cost. Given that formatting is a prototypical bikeshed situation, I know that 
> this is often subjective and thus really frustrating, and again, I'm truly 
> sorry for it, but I don't have a good solution :(
>
> > From what I understand as long as changes meet the guidelines set out about 
> > adding new options, changes have tests and have proper documentation then 
> > we should be free to approve and land those changes, The code owners have 
> > been given time like the rest of us to review and in the event they don't 
> > like the commit after the fact are free to submit a revision later or ask 
> > for further modifications. (make sure you check the done message on each of 
> > their review comments as you address them and ensure you leave a comment if 
> > you are not going to do the suggested change and why)
>
> I think there's consensus between folks responsible for clang-format that we 
> do specifically not want all features. The question is not only whether the 
> code follows guidelines and is documented and tested. All code incurs cost 
> and that has to be weighted carefully against the benefits.
>
> > As developers if we submit a revision we need to be prepared to look after 
> > that change and address and defects that come in, we don't want people 
> > driving by and dropping a patch that the code owners have to maintain, so 
> > with committing come responsibility.
> > 
> > I've personally been running with this patch in my clang format since 
> > @VelocityRa  took it over, and from what I can tell it works well, I too 
> > would like to see this landed.
> > 
> > With that said @VelocityRa and this comment, I would ask that you add 
> > something to the docs/ReleaseNotes.rst to show the addition of the new 
> > option to clang-format, and if you make that change, rebase (I doubt 
> > anything else was accepted in the meantime!) and send an updated revision I 
> > will add a LGTM and accept the review (for what that is worth), this will 
> > give @djasper  and @klimek  time to object to my proposal here.
> > 
> > Assuming others are in agreement, lets do something to move at least this 
> > forward.


Thank you for responsing.. It seems no one is is able to approve code here 
without your or @djasper's  input, of course we understand that is the ideal 
situation becau

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D28462#1405855 , @MyDeveloperDay 
wrote:

> In D28462#1405711 , @klimek wrote:
>
> > In D28462#1405360 , 
> > @MyDeveloperDay wrote:
> >
> > > I'm not a code owner here but we seem to be lacking people who are either 
> > > available, able, willing or allowed to approve code reviews for some of 
> > > the code in the clang-tooling (not just clang format).  The code owners 
> > > do an extremely good job at doing a deep review way beyond what many of 
> > > the rest of us are able to do..  They've been doing this for years having 
> > > written may of these tools in the first place, but I suspect other 
> > > external pressures and their phabricator activity suggests they have 
> > > limited time to contribute at present.
> > >
> > > As such code reviews for new features seem to sit unreviewed  and 
> > > unaccepted and yet the subscribers list and tokens, the pings and "whats 
> > > blocking this" comments would suggest that revisions such as these are 
> > > wanted but sit for weeks and months with no movement.
> > >
> > > It feels like we need more people reviewing code and approving code 
> > > changes otherwise these revisions bit rot or simply do not get into the 
> > > tool. It definitely will disenfranchise new developers from contributing, 
> > > if it takes months/years to get anything committed. But a shallow review 
> > > by someone with at least a little experience of this code is surely 
> > > better than no review at all. As such I'd like to volunteer to help 
> > > review some of these items.
> >
> >
> > I don't think shallow reviews before having the architectural questions 
> > tied down would help a lot, as they in my experience create more opposition 
> > of change later on.
> >
> > I fully agree that the state we're in is bad, and I'm truly sorry for us to 
> > have arrived here.
> >
> > > Not all areas of clang have this same problem, I see areas of code which 
> > > sometimes don't even get a review, where buddies review each others code 
> > > very quickly, or where the time from creation to commit can be measured 
> > > in hours or days, not weeks or months. I'm unclear what makes commits 
> > > here take so long and good improvements lost. As such I don't think we 
> > > get people contributing to fixing bugs either because the time taken to 
> > > get even a bug landed seems quite great.
> >
> > One of the differences is whether there is ahead-of-code-review-time 
> > agreement on the direction, and whether the code fits in well from an 
> > architectural point of view.
> >  In clang-format we specifically do not want to support all possible 
> > features, but carefully weigh the upside of features against maintenance 
> > cost. Given that formatting is a prototypical bikeshed situation, I know 
> > that this is often subjective and thus really frustrating, and again, I'm 
> > truly sorry for it, but I don't have a good solution :(
> >
> > > From what I understand as long as changes meet the guidelines set out 
> > > about adding new options, changes have tests and have proper 
> > > documentation then we should be free to approve and land those changes, 
> > > The code owners have been given time like the rest of us to review and in 
> > > the event they don't like the commit after the fact are free to submit a 
> > > revision later or ask for further modifications. (make sure you check the 
> > > done message on each of their review comments as you address them and 
> > > ensure you leave a comment if you are not going to do the suggested 
> > > change and why)
> >
> > I think there's consensus between folks responsible for clang-format that 
> > we do specifically not want all features. The question is not only whether 
> > the code follows guidelines and is documented and tested. All code incurs 
> > cost and that has to be weighted carefully against the benefits.
> >
> > > As developers if we submit a revision we need to be prepared to look 
> > > after that change and address and defects that come in, we don't want 
> > > people driving by and dropping a patch that the code owners have to 
> > > maintain, so with committing come responsibility.
> > > 
> > > I've personally been running with this patch in my clang format since 
> > > @VelocityRa  took it over, and from what I can tell it works well, I too 
> > > would like to see this landed.
> > > 
> > > With that said @VelocityRa and this comment, I would ask that you add 
> > > something to the docs/ReleaseNotes.rst to show the addition of the new 
> > > option to clang-format, and if you make that change, rebase (I doubt 
> > > anything else was accepted in the meantime!) and send an updated revision 
> > > I will add a LGTM and accept the review (for what that is worth), this 
> > > will give @djasper  and @klimek  time to object to my proposal here.
> > > 
> > > Assuming 

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D28462#1405360 , @MyDeveloperDay 
wrote:

> I'm not a code owner here but we seem to be lacking people who are either 
> available, able, willing or allowed to approve code reviews for some of the 
> code in the clang-tooling (not just clang format).  The code owners do an 
> extremely good job at doing a deep review way beyond what many of the rest of 
> us are able to do..  They've been doing this for years having written may of 
> these tools in the first place, but I suspect other external pressures and 
> their phabricator activity suggests they have limited time to contribute at 
> present.
>
> As such code reviews for new features seem to sit unreviewed  and unaccepted 
> and yet the subscribers list and tokens, the pings and "whats blocking this" 
> comments would suggest that revisions such as these are wanted but sit for 
> weeks and months with no movement.
>
> It feels like we need more people reviewing code and approving code changes 
> otherwise these revisions bit rot or simply do not get into the tool. It 
> definitely will disenfranchise new developers from contributing, if it takes 
> months/years to get anything committed. But a shallow review by someone with 
> at least a little experience of this code is surely better than no review at 
> all. As such I'd like to volunteer to help review some of these items.


I don't think shallow reviews before having the architectural questions tied 
down would help a lot, as they in my experience create more opposition of 
change later on.

I fully agree that the state we're in is bad, and I'm truly sorry for us to 
have arrived here.

> Not all areas of clang have this same problem, I see areas of code which 
> sometimes don't even get a review, where buddies review each others code very 
> quickly, or where the time from creation to commit can be measured in hours 
> or days, not weeks or months. I'm unclear what makes commits here take so 
> long and good improvements lost. As such I don't think we get people 
> contributing to fixing bugs either because the time taken to get even a bug 
> landed seems quite great.

One of the differences is whether there is ahead-of-code-review-time agreement 
on the direction, and whether the code fits in well from an architectural point 
of view.
In clang-format we specifically do not want to support all possible features, 
but carefully weigh the upside of features against maintenance cost. Given that 
formatting is a prototypical bikeshed situation, I know that this is often 
subjective and thus really frustrating, and again, I'm truly sorry for it, but 
I don't have a good solution :(

> From what I understand as long as changes meet the guidelines set out about 
> adding new options, changes have tests and have proper documentation then we 
> should be free to approve and land those changes, The code owners have been 
> given time like the rest of us to review and in the event they don't like the 
> commit after the fact are free to submit a revision later or ask for further 
> modifications. (make sure you check the done message on each of their review 
> comments as you address them and ensure you leave a comment if you are not 
> going to do the suggested change and why)

I think there's consensus between folks responsible for clang-format that we do 
specifically not want all features. The question is not only whether the code 
follows guidelines and is documented and tested. All code incurs cost and that 
has to be weighted carefully against the benefits.

> As developers if we submit a revision we need to be prepared to look after 
> that change and address and defects that come in, we don't want people 
> driving by and dropping a patch that the code owners have to maintain, so 
> with committing come responsibility.
> 
> I've personally been running with this patch in my clang format since 
> @VelocityRa  took it over, and from what I can tell it works well, I too 
> would like to see this landed.
> 
> With that said @VelocityRa and this comment, I would ask that you add 
> something to the docs/ReleaseNotes.rst to show the addition of the new option 
> to clang-format, and if you make that change, rebase (I doubt anything else 
> was accepted in the meantime!) and send an updated revision I will add a LGTM 
> and accept the review (for what that is worth), this will give @djasper  and 
> @klimek  time to object to my proposal here.
> 
> Assuming others are in agreement, lets do something to move at least this 
> forward.




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

https://reviews.llvm.org/D28462



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> Yes, we are, and as you said, it needs a set of patches to learn the code 
> base enough to contribute, to build up trust. This is easiest when the code 
> base is currently under active development, but many people consider 
> clang-format "good enough" minus bug fixes / new languages (like the C# 
> patch, which I haven't looked at).
> 
> Are you by chance going to be at the Euro LLVM dev meeting? If so, I'd be 
> happy to sit together with you to work on / talk through patches.

Alas I am not. I would however value your input on the C# patch, as I work 
running it through some larger C# code bases I've found some constructs that 
need additional changes. In the meantime my proposal still stands, I'd like to 
help, if that just means doing the leg work of ensuring patches have 
documentation, are formatted etc... but I don't want to railroad this revision 
with that discussion.


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

https://reviews.llvm.org/D28462



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

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[pierre gousseau via Phabricator via cfe-commits]

pgousseau added a comment.

In D57914#1405955 , @riccibruno wrote:

> More explicitly something like:
>
> in `Sanitizer.h`:
>
>   template  struct SanitizerMasks {
> static const SanitizerMask SomeMask;
> /* and so on for each mask*/
>   };
>  
>   template  const SanitizerMask SanitizerMasks::SomeMask = the 
> definition;
>
>
> And then you can write `SanitizerMasks<>::SomeMask` when you want to use this 
> mask.


Sounds good to me, I will try that thanks!


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

https://reviews.llvm.org/D57914



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

[PATCH] D58314: [Driver] Sync ARM behavior between clang-as and gas.

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

Thanks for the update. To summarise I'd like to keep the integrated and 
non-integrated assembler in synch for Linux like Android, even at the risk of 
adding an fpu when it isn't needed. I think that given how difficult it is to 
not use NEON, I think the mfloat-abi=soft guard you've put on will be 
sufficient. By the way, I'm hoping we aren't talking past each other with the 
default for armv7-a. I've tried to put as much of what I understand in the 
comments and I hope the answers make sense, or you'll be able to correct me 
where I'm wrong. If the timezone delayed comments aren't working well it may be 
worth finding me on IRC, I usually leave the office about 6:30pm but I can 
easily arrange a time and log on later if you wanted to discuss.

I also spotted something else on line 251 of  ARM.cpp with respect to Android 
that might be worth looking at. I've left a comment although it isn't related 
to this patch.




Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:277
+  }
+  return "";
+}

danalbert wrote:
> peter.smith wrote:
> > I'm a bit worried that we've changed the default behaviour for gnueabi[hf] 
> > targets here. 
> > For example with:
> > ```
> > .text
> > vmov.32 d13[1], r6 ; Needs VFPv2
> > vadd.i32 d0, d0, d0 ; Needs Neon
> > ```
> > I get with --target=armv7a-linux (no environment, -mfloat-abi will disable 
> > floating point, and neon)
> > ```
> > clang: warning: unknown platform, assuming -mfloat-abi=soft
> > neon.s:2:9: error: instruction requires: VFP2
> > vmov.32 d13[1],r6
> > ^
> > neon.s:3:9: error: instruction requires: NEON
> > vadd.i32 d0, d0, d0
> > ^
> > ```
> > With the target=armv7a-linux-gnueabi armv7a-linux-gnueabihf or explicitly 
> > adding -mfloat-abi=softfp the integrated assembler will happily assemble it.
> > GNU needs -mfpu=neon to assemble the file:
> > 
> > ```
> > arm-linux-gnueabihf-as -march=armv7-a neon.s 
> > neon.s: Assembler messages:
> > neon.s:2: Error: selected processor does not support ARM mode `vmov.32 
> > d13[1],r6'
> > neon.s:3: Error: selected processor does not support ARM mode `vadd.i32 
> > d0,d0,d0'
> > ```
> > It is a similar story for armv8 and crypto.
> > 
> > I think we should have something like:
> > ```
> > if (Triple.isLinux() && getARMSubArchVersionNumber(Triple) >= 8)
> >return "crypto-neon-fp-armv8";
> > if (Triple.isAndroid() || Triple.isLinux() && 
> > getARMSubArchVersionNumber(Triple) >= 7)
> > return "neon";
> > return "";
> > ```
> I suppose it depends on which direction you want the behavior change to go. I 
> assumed those samples _shouldn't_ assemble since they're not enabling NEON. 
> The fact that the direct `arm-linux-gnueabihf-as` doesn't enable NEON by 
> default makes me assume that NEON is not an assumed feature of the gnueabihf 
> environment.
> 
> It's not up to me whether NEON is available by default for ARMv7 for 
> non-Android, but I do think that the behavior should be consistent regardless 
> of the assembler being used. Right now we have no FPU by default with the 
> integrated assembler and NEON by default with GAS. This change makes GAS have 
> the same behavior as the integrated assembler, since I assume that is the 
> better traveled path and afaict is the one that has had more effort put in to 
> it.
> 
> If that's not right, I can change this so that the integrated assembler 
> _also_ gets FPU features that are not necessarily available for the given 
> architecture, but I wanted to clarify that that is what you're asking for 
> first.
> I suppose it depends on which direction you want the behavior change to go. I 
> assumed those samples _shouldn't_ assemble since they're not enabling NEON. 
> The fact that the direct arm-linux-gnueabihf-as doesn't enable NEON by 
> default makes me assume that NEON is not an assumed feature of the gnueabihf 
> environment.

It is true that LLVM and GNU have unfortunately chosen a different default for 
armv7-a and NEON and armv8-a and crypo. I agree that NEON is not an assumed 
feature of either a gnueabi or gnueabihf GCC toolchain. My understanding is 
that GNU chose to not include any extensions in the base architecture and LLVM 
chose the most common configuration for the default.

>  Right now we have no FPU by default with the integrated assembler and NEON 
> by default with GAS. This change makes GAS have the same behavior as the 
> integrated assembler, since I assume that is the better traveled path and 
> afaict is the one that has had more effort put in to it.

Are you sure that we have no FPU by default with the integrated assembler for 
armv7-a and armv8-a? I've put an explanation in the next comment that explains 
how it gets set even when there is no +neon on the -cc1 or -cc1as command line. 

> If that's not right, I can change this so that the integrated assembler 
> _also_ gets FPU features that are not necessarily available for the given 
> arch

[PATCH] D58492: [clangd] Add thread priority lowering for MacOS as well

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

macOS supports `pthread_setschedparam()` that we call on Linux, why not reuse 
the same code path?

https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/pthread_setschedparam.3.html


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58492



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

[PATCH] D58518: [HIP] change kernel stub name

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: t-tye, tra.

Add .stub to kernel stub function name so that it is different from kernel
name in device code. This is necessary to let debugger find correct symbol
for kernel


https://reviews.llvm.org/D58518

Files:
  lib/CodeGen/CGCUDANV.cpp
  test/CodeGenCUDA/device-stub.cu


Index: test/CodeGenCUDA/device-stub.cu
===
--- test/CodeGenCUDA/device-stub.cu
+++ test/CodeGenCUDA/device-stub.cu
@@ -145,7 +145,8 @@
 // Test that we build the correct number of calls to cudaSetupArgument followed
 // by a call to cudaLaunch.
 
-// LNX: define{{.*}}kernelfunc
+// CUDA-LABEL: define{{.*}}kernelfunc
+// HIP-LABEL: define{{.*}}@_Z10kernelfunciii.stub
 
 // New launch sequence stores arguments into local buffer and passes array of
 // pointers to them directly to cudaLaunchKernel
Index: lib/CodeGen/CGCUDANV.cpp
===
--- lib/CodeGen/CGCUDANV.cpp
+++ lib/CodeGen/CGCUDANV.cpp
@@ -227,6 +227,8 @@
 emitDeviceStubBodyNew(CGF, Args);
   else
 emitDeviceStubBodyLegacy(CGF, Args);
+  if (CGF.getLangOpts().HIP)
+CGF.CurFn->setName(CGF.CurFn->getName() + ".stub");
 }
 
 // CUDA 9.0+ uses new way to launch kernels. Parameters are packed in a local


Index: test/CodeGenCUDA/device-stub.cu
===
--- test/CodeGenCUDA/device-stub.cu
+++ test/CodeGenCUDA/device-stub.cu
@@ -145,7 +145,8 @@
 // Test that we build the correct number of calls to cudaSetupArgument followed
 // by a call to cudaLaunch.
 
-// LNX: define{{.*}}kernelfunc
+// CUDA-LABEL: define{{.*}}kernelfunc
+// HIP-LABEL: define{{.*}}@_Z10kernelfunciii.stub
 
 // New launch sequence stores arguments into local buffer and passes array of
 // pointers to them directly to cudaLaunchKernel
Index: lib/CodeGen/CGCUDANV.cpp
===
--- lib/CodeGen/CGCUDANV.cpp
+++ lib/CodeGen/CGCUDANV.cpp
@@ -227,6 +227,8 @@
 emitDeviceStubBodyNew(CGF, Args);
   else
 emitDeviceStubBodyLegacy(CGF, Args);
+  if (CGF.getLangOpts().HIP)
+CGF.CurFn->setName(CGF.CurFn->getName() + ".stub");
 }
 
 // CUDA 9.0+ uses new way to launch kernels. Parameters are packed in a local
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57219: [Fixed Point Arithmetic] Fixed Point Comparisons

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a subscriber: bevinh.
leonardchan added a comment.
Herald added a subscriber: jdoerfert.

*ping* @bevinh @rjmccall @bjope Any other comments on this patch?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57219



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

[PATCH] D58404: [clang-format] Add basic support for formatting C# files

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Note to self...Underdevelopment:

- adding support for => (JSBigArrow) for C# Lambas
- adding support for $"liternal {string} with arguments"


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

https://reviews.llvm.org/D58404



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

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: include/clang/AST/Decl.h:1482
 
+  bool getIsArgumentModified() const {
+return IsArgumentModified;

There should be a comment here.
The style in this class appears to omit the 'get' word from the name of the 
getter, so `isArgumentModified`  for the method name would look more consistent.



Comment at: lib/Sema/SemaExpr.cpp:11282
+static void EmitArgumentsValueModification(Expr *E) {
+  if (const DeclRefExpr *LHSDeclRef =
+  dyn_cast(E->IgnoreParenCasts()))

Does this fit on one line if you use `const auto *LHSDeclRef ...`? Given you 
have the dyn_cast on the RHS there's no real need to give the type name twice.


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

https://reviews.llvm.org/D58035



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

[PATCH] D58292: Add support for importing ChooseExpr AST nodes.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder updated this revision to Diff 187821.
tmroeder marked an inline comment as done.
tmroeder added a comment.

Changed the CondIsTrue RHS as suggested.

Left the `auto` based on the conclusion of the discussion.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58292

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  test/ASTMerge/choose-expr/Inputs/choose.c
  test/ASTMerge/choose-expr/test.c
  unittests/AST/ASTImporterTest.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -754,6 +754,11 @@
   EXPECT_TRUE(matches("int* i = nullptr;", cxxNullPtrLiteralExpr()));
 }
 
+TEST(Matcher, ChooseExpr) {
+  EXPECT_TRUE(matchesC("void f() { (void)__builtin_choose_expr(1, 2, 3); }",
+   chooseExpr()));
+}
+
 TEST(Matcher, GNUNullExpr) {
   EXPECT_TRUE(matches("int* i = __null;", gnuNullExpr()));
 }
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -563,6 +563,15 @@
   stringLiteral(hasType(asString("const char [7]"));
 }
 
+TEST_P(ImportExpr, ImportChooseExpr) {
+  MatchVerifier Verifier;
+
+  testImport(
+"void declToImport() { __builtin_choose_expr(1, 2, 3); }",
+Lang_C, "", Lang_C, Verifier,
+functionDecl(hasDescendant(chooseExpr(;
+}
+
 TEST_P(ImportExpr, ImportGNUNullExpr) {
   MatchVerifier Verifier;
   testImport(
Index: test/ASTMerge/choose-expr/test.c
===
--- /dev/null
+++ test/ASTMerge/choose-expr/test.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c11 -emit-pch -o %t.ast %S/Inputs/choose.c
+// RUN: %clang_cc1 -std=c11 -ast-merge %t.ast -fsyntax-only -verify %s
+// expected-no-diagnostics
+
Index: test/ASTMerge/choose-expr/Inputs/choose.c
===
--- /dev/null
+++ test/ASTMerge/choose-expr/Inputs/choose.c
@@ -0,0 +1,2 @@
+_Static_assert(__builtin_choose_expr(1, 1, 0), "Incorrect semantics of __builtin_choose_expr");
+_Static_assert(__builtin_choose_expr(0, 0, 1), "Incorrect semantics of __builtin_choose_expr");
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -147,6 +147,7 @@
   REGISTER_MATCHER(caseStmt);
   REGISTER_MATCHER(castExpr);
   REGISTER_MATCHER(characterLiteral);
+  REGISTER_MATCHER(chooseExpr);
   REGISTER_MATCHER(classTemplateDecl);
   REGISTER_MATCHER(classTemplateSpecializationDecl);
   REGISTER_MATCHER(complexType);
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -727,6 +727,7 @@
 compoundLiteralExpr;
 const internal::VariadicDynCastAllOfMatcher
 cxxNullPtrLiteralExpr;
+const internal::VariadicDynCastAllOfMatcher chooseExpr;
 const internal::VariadicDynCastAllOfMatcher gnuNullExpr;
 const internal::VariadicDynCastAllOfMatcher atomicExpr;
 const internal::VariadicDynCastAllOfMatcher stmtExpr;
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -552,6 +552,7 @@
 // Importing expressions
 ExpectedStmt VisitExpr(Expr *E);
 ExpectedStmt VisitVAArgExpr(VAArgExpr *E);
+ExpectedStmt VisitChooseExpr(ChooseExpr *E);
 ExpectedStmt VisitGNUNullExpr(GNUNullExpr *E);
 ExpectedStmt VisitPredefinedExpr(PredefinedExpr *E);
 ExpectedStmt VisitDeclRefExpr(DeclRefExpr *E);
@@ -6135,6 +6136,33 @@
   E->isMicrosoftABI());
 }
 
+ExpectedStmt ASTNodeImporter::VisitChooseExpr(ChooseExpr *E) {
+  auto Imp = importSeq(E->getCond(), E->getLHS(), E->getRHS(),
+   E->getBuiltinLoc(), E->getRParenLoc(), E->getType());
+  if (!Imp)
+return Imp.takeError();
+
+  Expr *ToCond;
+  Expr *ToLHS;
+  Expr *ToRHS;
+  SourceLocation ToBuiltinLoc, ToRParenLoc;
+  QualType ToType;
+  std::tie(ToCond, ToLHS, ToRHS, ToBuiltinLoc, ToRParenLoc, ToType) = *Imp;
+
+  ExprValueKind VK = E->getValueKind();
+  ExprObjectKind OK = E->getObjectKind();
+
+  bool TypeDependent = ToCond->isTypeDependent();
+  bool ValueDependent = ToCond->isValueDependent();
+
+  // The value of CondIsTrue only matters if the value is not
+  // condition-dependent.
+  bool CondIsTrue = E->isConditionDependent() && E->isConditionTrue();
+
+  return new (Importer.

[PATCH] D58292: Add support for importing ChooseExpr AST nodes.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder updated this revision to Diff 187823.
tmroeder added a comment.

Fix a mistake in the comment.

CondIsTrue only matters if the value *is* condition dependent, not *is not* 
condition dependent.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58292

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  test/ASTMerge/choose-expr/Inputs/choose.c
  test/ASTMerge/choose-expr/test.c
  unittests/AST/ASTImporterTest.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -754,6 +754,11 @@
   EXPECT_TRUE(matches("int* i = nullptr;", cxxNullPtrLiteralExpr()));
 }
 
+TEST(Matcher, ChooseExpr) {
+  EXPECT_TRUE(matchesC("void f() { (void)__builtin_choose_expr(1, 2, 3); }",
+   chooseExpr()));
+}
+
 TEST(Matcher, GNUNullExpr) {
   EXPECT_TRUE(matches("int* i = __null;", gnuNullExpr()));
 }
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -563,6 +563,15 @@
   stringLiteral(hasType(asString("const char [7]"));
 }
 
+TEST_P(ImportExpr, ImportChooseExpr) {
+  MatchVerifier Verifier;
+
+  testImport(
+"void declToImport() { __builtin_choose_expr(1, 2, 3); }",
+Lang_C, "", Lang_C, Verifier,
+functionDecl(hasDescendant(chooseExpr(;
+}
+
 TEST_P(ImportExpr, ImportGNUNullExpr) {
   MatchVerifier Verifier;
   testImport(
Index: test/ASTMerge/choose-expr/test.c
===
--- /dev/null
+++ test/ASTMerge/choose-expr/test.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c11 -emit-pch -o %t.ast %S/Inputs/choose.c
+// RUN: %clang_cc1 -std=c11 -ast-merge %t.ast -fsyntax-only -verify %s
+// expected-no-diagnostics
+
Index: test/ASTMerge/choose-expr/Inputs/choose.c
===
--- /dev/null
+++ test/ASTMerge/choose-expr/Inputs/choose.c
@@ -0,0 +1,2 @@
+_Static_assert(__builtin_choose_expr(1, 1, 0), "Incorrect semantics of __builtin_choose_expr");
+_Static_assert(__builtin_choose_expr(0, 0, 1), "Incorrect semantics of __builtin_choose_expr");
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -147,6 +147,7 @@
   REGISTER_MATCHER(caseStmt);
   REGISTER_MATCHER(castExpr);
   REGISTER_MATCHER(characterLiteral);
+  REGISTER_MATCHER(chooseExpr);
   REGISTER_MATCHER(classTemplateDecl);
   REGISTER_MATCHER(classTemplateSpecializationDecl);
   REGISTER_MATCHER(complexType);
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -727,6 +727,7 @@
 compoundLiteralExpr;
 const internal::VariadicDynCastAllOfMatcher
 cxxNullPtrLiteralExpr;
+const internal::VariadicDynCastAllOfMatcher chooseExpr;
 const internal::VariadicDynCastAllOfMatcher gnuNullExpr;
 const internal::VariadicDynCastAllOfMatcher atomicExpr;
 const internal::VariadicDynCastAllOfMatcher stmtExpr;
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -552,6 +552,7 @@
 // Importing expressions
 ExpectedStmt VisitExpr(Expr *E);
 ExpectedStmt VisitVAArgExpr(VAArgExpr *E);
+ExpectedStmt VisitChooseExpr(ChooseExpr *E);
 ExpectedStmt VisitGNUNullExpr(GNUNullExpr *E);
 ExpectedStmt VisitPredefinedExpr(PredefinedExpr *E);
 ExpectedStmt VisitDeclRefExpr(DeclRefExpr *E);
@@ -6135,6 +6136,32 @@
   E->isMicrosoftABI());
 }
 
+ExpectedStmt ASTNodeImporter::VisitChooseExpr(ChooseExpr *E) {
+  auto Imp = importSeq(E->getCond(), E->getLHS(), E->getRHS(),
+   E->getBuiltinLoc(), E->getRParenLoc(), E->getType());
+  if (!Imp)
+return Imp.takeError();
+
+  Expr *ToCond;
+  Expr *ToLHS;
+  Expr *ToRHS;
+  SourceLocation ToBuiltinLoc, ToRParenLoc;
+  QualType ToType;
+  std::tie(ToCond, ToLHS, ToRHS, ToBuiltinLoc, ToRParenLoc, ToType) = *Imp;
+
+  ExprValueKind VK = E->getValueKind();
+  ExprObjectKind OK = E->getObjectKind();
+
+  bool TypeDependent = ToCond->isTypeDependent();
+  bool ValueDependent = ToCond->isValueDependent();
+
+  // The value of CondIsTrue only matters if the value is condition-dependent.
+  bool CondIsTrue = E->isConditionDependent() && E->isConditionTrue();
+
+  return new (Importer.getToContext())
+  Ch

[PATCH] D58186: Sync some doc changes ClangFormatStyleOptions.rst with doc comments in `Format.h`

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

In D58186#1405687 , @sylvestre.ledru 
wrote:

> Do you have permissions on the repo?


No, I would need someone to commit this for me. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58186



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

[PATCH] D58292: Add support for importing ChooseExpr AST nodes.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58292



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

[PATCH] D58292: Add support for importing ChooseExpr AST nodes.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder added inline comments.



Comment at: lib/AST/ASTImporter.cpp:6160
+  // condition-dependent.
+  bool CondIsTrue = false;
+  if (!E->isConditionDependent())

aaron.ballman wrote:
> a_sidorin wrote:
> > aaron.ballman wrote:
> > > `bool CondIsTrue = E->isConditionDependent() ? false : 
> > > E->isConditionTrue();`
> > `bool CondIsTrue = E->isConditionDependent() && E->isConditionTrue();`?
> I like that even better than my suggestion. :-)
Wait, this doesn't have the same truth table as my original code.

let `CD = E->isConditionDependent()`
let `CT = E->isConditionTrue()`

in

```
bool CondIsTrue = false;
if (!CD)
  CondIsTrue = CT;
```

has the table for `CondIsTrue`:

| `CD` | `CT` |  `CondIsTrue` |
| T | T | F |
| T | F | F |
| F | T | T |
| F | F | F |
i.e., if CD is true, then CondIsTrue is always false. Otherwise, it's the value 
of CT.

The suggested line has the truth table

| `CD` | `CT` |  `CondIsTrue` |
| T | T | T |
| T | F | F |
| F | T | F |
| F | F | F |

That is, the effect of CD is swapped.

Aaron's suggestion matches my original table. I based my code on 
include/clang/AST/Expr.h line 4032, which asserts !isConditionDependent() in 
the implementation of isConditionTrue.

I realized this after I "fixed" my comment to match the implementation change. 
Am I missing something? Or is the assertion in Expr.h wrong? I think this 
should be

```
bool CondIsTrue = !E->isConditionDependent() && E->isConditionTrue();
```

I've changed my code to that and reverted the comment change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58292



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

[PATCH] D58292: Add support for importing ChooseExpr AST nodes.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder updated this revision to Diff 187827.
tmroeder marked an inline comment as done.
tmroeder added a comment.

Reverted to the original semantics of CondIsTrue


Repository:
  rC Clang

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

https://reviews.llvm.org/D58292

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  test/ASTMerge/choose-expr/Inputs/choose.c
  test/ASTMerge/choose-expr/test.c
  unittests/AST/ASTImporterTest.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -754,6 +754,11 @@
   EXPECT_TRUE(matches("int* i = nullptr;", cxxNullPtrLiteralExpr()));
 }
 
+TEST(Matcher, ChooseExpr) {
+  EXPECT_TRUE(matchesC("void f() { (void)__builtin_choose_expr(1, 2, 3); }",
+   chooseExpr()));
+}
+
 TEST(Matcher, GNUNullExpr) {
   EXPECT_TRUE(matches("int* i = __null;", gnuNullExpr()));
 }
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -563,6 +563,15 @@
   stringLiteral(hasType(asString("const char [7]"));
 }
 
+TEST_P(ImportExpr, ImportChooseExpr) {
+  MatchVerifier Verifier;
+
+  testImport(
+"void declToImport() { __builtin_choose_expr(1, 2, 3); }",
+Lang_C, "", Lang_C, Verifier,
+functionDecl(hasDescendant(chooseExpr(;
+}
+
 TEST_P(ImportExpr, ImportGNUNullExpr) {
   MatchVerifier Verifier;
   testImport(
Index: test/ASTMerge/choose-expr/test.c
===
--- /dev/null
+++ test/ASTMerge/choose-expr/test.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c11 -emit-pch -o %t.ast %S/Inputs/choose.c
+// RUN: %clang_cc1 -std=c11 -ast-merge %t.ast -fsyntax-only -verify %s
+// expected-no-diagnostics
+
Index: test/ASTMerge/choose-expr/Inputs/choose.c
===
--- /dev/null
+++ test/ASTMerge/choose-expr/Inputs/choose.c
@@ -0,0 +1,2 @@
+_Static_assert(__builtin_choose_expr(1, 1, 0), "Incorrect semantics of __builtin_choose_expr");
+_Static_assert(__builtin_choose_expr(0, 0, 1), "Incorrect semantics of __builtin_choose_expr");
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -147,6 +147,7 @@
   REGISTER_MATCHER(caseStmt);
   REGISTER_MATCHER(castExpr);
   REGISTER_MATCHER(characterLiteral);
+  REGISTER_MATCHER(chooseExpr);
   REGISTER_MATCHER(classTemplateDecl);
   REGISTER_MATCHER(classTemplateSpecializationDecl);
   REGISTER_MATCHER(complexType);
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -727,6 +727,7 @@
 compoundLiteralExpr;
 const internal::VariadicDynCastAllOfMatcher
 cxxNullPtrLiteralExpr;
+const internal::VariadicDynCastAllOfMatcher chooseExpr;
 const internal::VariadicDynCastAllOfMatcher gnuNullExpr;
 const internal::VariadicDynCastAllOfMatcher atomicExpr;
 const internal::VariadicDynCastAllOfMatcher stmtExpr;
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -552,6 +552,7 @@
 // Importing expressions
 ExpectedStmt VisitExpr(Expr *E);
 ExpectedStmt VisitVAArgExpr(VAArgExpr *E);
+ExpectedStmt VisitChooseExpr(ChooseExpr *E);
 ExpectedStmt VisitGNUNullExpr(GNUNullExpr *E);
 ExpectedStmt VisitPredefinedExpr(PredefinedExpr *E);
 ExpectedStmt VisitDeclRefExpr(DeclRefExpr *E);
@@ -6135,6 +6136,33 @@
   E->isMicrosoftABI());
 }
 
+ExpectedStmt ASTNodeImporter::VisitChooseExpr(ChooseExpr *E) {
+  auto Imp = importSeq(E->getCond(), E->getLHS(), E->getRHS(),
+   E->getBuiltinLoc(), E->getRParenLoc(), E->getType());
+  if (!Imp)
+return Imp.takeError();
+
+  Expr *ToCond;
+  Expr *ToLHS;
+  Expr *ToRHS;
+  SourceLocation ToBuiltinLoc, ToRParenLoc;
+  QualType ToType;
+  std::tie(ToCond, ToLHS, ToRHS, ToBuiltinLoc, ToRParenLoc, ToType) = *Imp;
+
+  ExprValueKind VK = E->getValueKind();
+  ExprObjectKind OK = E->getObjectKind();
+
+  bool TypeDependent = ToCond->isTypeDependent();
+  bool ValueDependent = ToCond->isValueDependent();
+
+  // The value of CondIsTrue only matters if the value is not
+  // condition-dependent.
+  bool CondIsTrue = !E->isConditionDependent() && E->isConditionTrue();
+
+  return new (Importer.getToContext())
+  ChooseExpr(ToBuiltinLoc, ToC

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

[Vedant Kumar via Phabricator via cfe-commits]

vsk added reviewers: bricci, rsmith.
vsk added a comment.

+ rsmith, + bricci for review.

I was under the impression that space inside VarDecl was quite constrained. 
Pardon the likely naive question, but: is there any way to make the 
representation more compact (maybe sneak a bit into ParmVarDeclBitfields)?


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

https://reviews.llvm.org/D58035



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

[PATCH] D58518: [HIP] change kernel stub name

[Artem Belevich via Phabricator via cfe-commits]

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

My guess is that this is needed because HIP debugger can see symbols from both 
host and device executables at the same time. Is that so?

If that's the case, I guess HIP may have similar naming problem for `__host__ 
__device__ foo()` if it's used on both host and device.




Comment at: lib/CodeGen/CGCUDANV.cpp:230-231
 emitDeviceStubBodyLegacy(CGF, Args);
+  if (CGF.getLangOpts().HIP)
+CGF.CurFn->setName(CGF.CurFn->getName() + ".stub");
 }

It may be worth adding a comment why kernel stub in HIP needs a different name.


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

https://reviews.llvm.org/D58518



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

[PATCH] D58292: Add support for importing ChooseExpr AST nodes.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/AST/ASTImporter.cpp:6160
+  // condition-dependent.
+  bool CondIsTrue = false;
+  if (!E->isConditionDependent())

tmroeder wrote:
> aaron.ballman wrote:
> > a_sidorin wrote:
> > > aaron.ballman wrote:
> > > > `bool CondIsTrue = E->isConditionDependent() ? false : 
> > > > E->isConditionTrue();`
> > > `bool CondIsTrue = E->isConditionDependent() && E->isConditionTrue();`?
> > I like that even better than my suggestion. :-)
> Wait, this doesn't have the same truth table as my original code.
> 
> let `CD = E->isConditionDependent()`
> let `CT = E->isConditionTrue()`
> 
> in
> 
> ```
> bool CondIsTrue = false;
> if (!CD)
>   CondIsTrue = CT;
> ```
> 
> has the table for `CondIsTrue`:
> 
> | `CD` | `CT` |  `CondIsTrue` |
> | T | T | F |
> | T | F | F |
> | F | T | T |
> | F | F | F |
> i.e., if CD is true, then CondIsTrue is always false. Otherwise, it's the 
> value of CT.
> 
> The suggested line has the truth table
> 
> | `CD` | `CT` |  `CondIsTrue` |
> | T | T | T |
> | T | F | F |
> | F | T | F |
> | F | F | F |
> 
> That is, the effect of CD is swapped.
> 
> Aaron's suggestion matches my original table. I based my code on 
> include/clang/AST/Expr.h line 4032, which asserts !isConditionDependent() in 
> the implementation of isConditionTrue.
> 
> I realized this after I "fixed" my comment to match the implementation 
> change. Am I missing something? Or is the assertion in Expr.h wrong? I think 
> this should be
> 
> ```
> bool CondIsTrue = !E->isConditionDependent() && E->isConditionTrue();
> ```
> 
> I've changed my code to that and reverted the comment change.
Good catch -- I think my eyes just missed the change in logic. Perhaps we 
should add a test case that exercises this?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58292



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

r354608 - [test] Fix typo: 's/ ot / to /' [NFC]

[Mandeep Singh Grang via cfe-commits]

Author: mgrang
Date: Thu Feb 21 11:11:15 2019
New Revision: 354608

URL: http://llvm.org/viewvc/llvm-project?rev=354608&view=rev
Log:
[test] Fix typo: 's/ ot / to /' [NFC]

Modified:
cfe/trunk/test/CodeGenCXX/cxx11-special-members.cpp

Modified: cfe/trunk/test/CodeGenCXX/cxx11-special-members.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx11-special-members.cpp?rev=354608&r1=354607&r2=354608&view=diff
==
--- cfe/trunk/test/CodeGenCXX/cxx11-special-members.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/cxx11-special-members.cpp Thu Feb 21 11:11:15 2019
@@ -40,7 +40,7 @@ void f3() {
   D b;
 }
 // Trivial default ctor, might or might not be defined, but we must not expect
-// someone else ot define it.
+// someone else to define it.
 // CHECK-NOT: declare {{.*}} @_ZN1CILi0EEC1Ev
 // CHECK: define {{.*}} @_ZN1DC1Ev
 


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

[PATCH] D58504: [OpenCL][8.0.0 Release] Notes for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: docs/ReleaseNotes.rst:228
 
-OpenCL C Language Changes in Clang
+OpenCL Language Changes in Clang
 --

AlexeySotkin wrote:
> AlexeySotkin wrote:
> > Why the "C" is removed ?
> Should we call the section like: "OpenCL Support in Clang"?
I would like to leave "Changes" in as it described the difference in this 
release. And I think "Language" should stay to be more precise that this 
doesn't cover libraries or runtime.

So I was think of either:
`OpenCL Kernel Language Change in Clang`
 or
`'OpenCL C/C++ Language Changes in Clang'`

I am a bit worried about the last one since it might link to the OpenCL C++ 
spec... even though it was used in the past release notes... but now that the 
direction has been changed I am not sure. Any opinion?


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

https://reviews.llvm.org/D58504



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

[PATCH] D58518: [HIP] change kernel stub name

[Yaxun Liu via Phabricator via cfe-commits]

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

In D58518#1406124 , @tra wrote:

> My guess is that this is needed because HIP debugger can see symbols from 
> both host and device executables at the same time. Is that so?
>
> If that's the case, I guess HIP may have similar naming problem for `__host__ 
> __device__ foo()` if it's used on both host and device.


Probably, will fix it in seperate patch if it is true.




Comment at: lib/CodeGen/CGCUDANV.cpp:230-231
 emitDeviceStubBodyLegacy(CGF, Args);
+  if (CGF.getLangOpts().HIP)
+CGF.CurFn->setName(CGF.CurFn->getName() + ".stub");
 }

tra wrote:
> It may be worth adding a comment why kernel stub in HIP needs a different 
> name.
will do when commit


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

https://reviews.llvm.org/D58518



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

[PATCH] D58509: [CodeGen] Fix string literal address space casting.

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58509



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

[PATCH] D58523: [OpenMP 5.0] Parsing/sema support for to and from clauses with mapper modifier

[Lingda Li via Phabricator via cfe-commits]

lildmh created this revision.
lildmh added reviewers: ABataev, hfinkel, Meinersbur, kkwli0.
lildmh added a project: OpenMP.
Herald added subscribers: cfe-commits, jdoerfert, guansong.
Herald added a project: clang.

This patch implements the parsing and sema support for OpenMP to and from 
clauses with potential user-defined mappers attached. User defined mapper is a 
new feature in OpenMP 5.0. A to/from clause can have an explicit or implicit 
associated mapper, which instructs the compiler to generate and use customized 
mapping functions. An example is shown below:

  struct S { int len; int *d; };
  #pragma omp declare mapper(id: struct S s) map(s, s.d[0:s.len])
  struct S ss;
  #pragma omp target update to(mapper(id): ss) // use the mapper with name 'id' 
to map ss to device


Repository:
  rC Clang

https://reviews.llvm.org/D58523

Files:
  include/clang/AST/OpenMPClause.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Basic/OpenMPKinds.h
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/OpenMPClause.cpp
  lib/Basic/OpenMPKinds.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/OpenMP/declare_mapper_ast_print.c
  test/OpenMP/declare_mapper_ast_print.cpp
  test/OpenMP/declare_mapper_codegen.cpp
  test/OpenMP/declare_mapper_messages.c
  test/OpenMP/declare_mapper_messages.cpp

Index: test/OpenMP/declare_mapper_messages.cpp
===
--- test/OpenMP/declare_mapper_messages.cpp
+++ test/OpenMP/declare_mapper_messages.cpp
@@ -29,7 +29,7 @@
 #pragma omp declare mapper(bb: vec v) private(v)// expected-error {{expected at least one clause on '#pragma omp declare mapper' directive}} // expected-error {{unexpected OpenMP clause 'private' in directive '#pragma omp declare mapper'}}
 #pragma omp declare mapper(cc: vec v) map(v) (  // expected-warning {{extra tokens at the end of '#pragma omp declare mapper' are ignored}}
 
-#pragma omp declare mapper(++: vec v) map(v.len)// expected-error {{illegal identifier on 'omp declare mapper' directive}}
+#pragma omp declare mapper(++: vec v) map(v.len)// expected-error {{illegal OpenMP user-defined mapper identifier}}
 #pragma omp declare mapper(id1: vec v) map(v.len, temp) // expected-error {{only variable v is allowed in map clauses of this 'omp declare mapper' directive}}
 #pragma omp declare mapper(default : vec kk) map(kk.data[0:2])  // expected-note {{previous definition is here}}
 #pragma omp declare mapper(vec v) map(v.len)// expected-error {{redefinition of user-defined mapper for type 'vec' with name 'default'}}
@@ -74,9 +74,9 @@
   {}
 #pragma omp target map(mapper(ab) :vv)  // expected-error {{missing map type}} expected-error {{cannot find a valid user-defined mapper for type 'vec' with name 'ab'}}
   {}
-#pragma omp target map(mapper(N2::) :vv)// expected-error {{use of undeclared identifier 'N2'}} expected-error {{illegal identifier on 'omp declare mapper' directive}}
+#pragma omp target map(mapper(N2::) :vv)// expected-error {{use of undeclared identifier 'N2'}} expected-error {{illegal OpenMP user-defined mapper identifier}}
   {}
-#pragma omp target map(mapper(N1::) :vv)// expected-error {{illegal identifier on 'omp declare mapper' directive}}
+#pragma omp target map(mapper(N1::) :vv)// expected-error {{illegal OpenMP user-defined mapper identifier}}
   {}
 #pragma omp target map(mapper(aa) :vv)  // expected-error {{missing map type}}
   {}
@@ -86,6 +86,32 @@
   {}
 #pragma omp target map(mapper(N1::stack::id) to:vv)
   {}
+
+#pragma omp target update to(mapper)// expected-error {{expected '(' after 'mapper'}} expected-error {{expected expression}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper()   // expected-error {{illegal OpenMP user-defined mapper identifier}} expected-error {{expected expression}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper:vv) // expected-error {{expected '(' after 'mapper'}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper(:vv)// expected-error {{illegal OpenMP user-defined mapper identifier}} expected-err

[PATCH] D58518: [HIP] change kernel stub name

[Tony Tye via Phabricator via cfe-commits]

t-tye added a comment.

Yes this relates to supporting the debugger.

For the same function being present on both host and device, having the same 
name is correct as the debugger must set a breakpoint at both places. This is 
similar to needing to set a breakpoint at every place a function is inlined.


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

https://reviews.llvm.org/D58518



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

r354610 - [CodeGen] Fix string literal address space casting.

[Michael Liao via cfe-commits]

Author: hliao
Date: Thu Feb 21 11:40:20 2019
New Revision: 354610

URL: http://llvm.org/viewvc/llvm-project?rev=354610&view=rev
Log:
[CodeGen] Fix string literal address space casting.

Summary:
- If a string literal is reused directly, need to add necessary address
  space casting if the target requires that.

Reviewers: yaxunl

Subscribers: jvesely, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGenCXX/amdgcn-string-literal.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=354610&r1=354609&r2=354610&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Feb 21 11:40:20 2019
@@ -4522,7 +4522,8 @@ CodeGenModule::GetAddrOfConstantStringFr
 if (auto GV = *Entry) {
   if (Alignment.getQuantity() > GV->getAlignment())
 GV->setAlignment(Alignment.getQuantity());
-  return ConstantAddress(GV, Alignment);
+  return ConstantAddress(castStringLiteralToDefaultAddressSpace(*this, GV),
+ Alignment);
 }
   }
 
@@ -4584,7 +4585,8 @@ ConstantAddress CodeGenModule::GetAddrOf
 if (auto GV = *Entry) {
   if (Alignment.getQuantity() > GV->getAlignment())
 GV->setAlignment(Alignment.getQuantity());
-  return ConstantAddress(GV, Alignment);
+  return ConstantAddress(castStringLiteralToDefaultAddressSpace(*this, GV),
+ Alignment);
 }
   }
 

Modified: cfe/trunk/test/CodeGenCXX/amdgcn-string-literal.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/amdgcn-string-literal.cpp?rev=354610&r1=354609&r2=354610&view=diff
==
--- cfe/trunk/test/CodeGenCXX/amdgcn-string-literal.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/amdgcn-string-literal.cpp Thu Feb 21 11:40:20 2019
@@ -14,7 +14,7 @@ void g(const char* p);
 // CHECK-LABEL: define void @_Z1fv()
 void f() {
   const char* l_str = "l_str";
-  
+
   // CHECK: call void @llvm.memcpy.p0i8.p4i8.i64
   char l_array[] = "l_array";
 
@@ -26,3 +26,9 @@ void f() {
   const char* p = g_str;
   g(p);
 }
+
+// CHECK-LABEL: define void @_Z1ev
+void e() {
+  g("string literal");
+  g("string literal");
+}


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

[PATCH] D58509: [CodeGen] Fix string literal address space casting.

[Michael Liao via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC354610: [CodeGen] Fix string literal address space casting. 
(authored by hliao, committed by ).
Herald added a subscriber: ebevhan.

Changed prior to commit:
  https://reviews.llvm.org/D58509?vs=187797&id=187834#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58509

Files:
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGenCXX/amdgcn-string-literal.cpp


Index: test/CodeGenCXX/amdgcn-string-literal.cpp
===
--- test/CodeGenCXX/amdgcn-string-literal.cpp
+++ test/CodeGenCXX/amdgcn-string-literal.cpp
@@ -14,7 +14,7 @@
 // CHECK-LABEL: define void @_Z1fv()
 void f() {
   const char* l_str = "l_str";
-  
+
   // CHECK: call void @llvm.memcpy.p0i8.p4i8.i64
   char l_array[] = "l_array";
 
@@ -26,3 +26,9 @@
   const char* p = g_str;
   g(p);
 }
+
+// CHECK-LABEL: define void @_Z1ev
+void e() {
+  g("string literal");
+  g("string literal");
+}
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -4522,7 +4522,8 @@
 if (auto GV = *Entry) {
   if (Alignment.getQuantity() > GV->getAlignment())
 GV->setAlignment(Alignment.getQuantity());
-  return ConstantAddress(GV, Alignment);
+  return ConstantAddress(castStringLiteralToDefaultAddressSpace(*this, GV),
+ Alignment);
 }
   }
 
@@ -4584,7 +4585,8 @@
 if (auto GV = *Entry) {
   if (Alignment.getQuantity() > GV->getAlignment())
 GV->setAlignment(Alignment.getQuantity());
-  return ConstantAddress(GV, Alignment);
+  return ConstantAddress(castStringLiteralToDefaultAddressSpace(*this, GV),
+ Alignment);
 }
   }
 


Index: test/CodeGenCXX/amdgcn-string-literal.cpp
===
--- test/CodeGenCXX/amdgcn-string-literal.cpp
+++ test/CodeGenCXX/amdgcn-string-literal.cpp
@@ -14,7 +14,7 @@
 // CHECK-LABEL: define void @_Z1fv()
 void f() {
   const char* l_str = "l_str";
-  
+
   // CHECK: call void @llvm.memcpy.p0i8.p4i8.i64
   char l_array[] = "l_array";
 
@@ -26,3 +26,9 @@
   const char* p = g_str;
   g(p);
 }
+
+// CHECK-LABEL: define void @_Z1ev
+void e() {
+  g("string literal");
+  g("string literal");
+}
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -4522,7 +4522,8 @@
 if (auto GV = *Entry) {
   if (Alignment.getQuantity() > GV->getAlignment())
 GV->setAlignment(Alignment.getQuantity());
-  return ConstantAddress(GV, Alignment);
+  return ConstantAddress(castStringLiteralToDefaultAddressSpace(*this, GV),
+ Alignment);
 }
   }
 
@@ -4584,7 +4585,8 @@
 if (auto GV = *Entry) {
   if (Alignment.getQuantity() > GV->getAlignment())
 GV->setAlignment(Alignment.getQuantity());
-  return ConstantAddress(GV, Alignment);
+  return ConstantAddress(castStringLiteralToDefaultAddressSpace(*this, GV),
+ Alignment);
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56411: [CUDA][HIP][Sema] Fix template kernel with function as template parameter

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 187832.
yaxunl added a comment.

I would like to fix the validation issue only and leave the overload resolution 
issue for future.


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

https://reviews.llvm.org/D56411

Files:
  lib/Sema/SemaCUDA.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaCUDA/call-device-fn-from-host.cu
  test/SemaCUDA/call-host-fn-from-device.cu


Index: test/SemaCUDA/call-host-fn-from-device.cu
===
--- test/SemaCUDA/call-host-fn-from-device.cu
+++ test/SemaCUDA/call-host-fn-from-device.cu
@@ -56,14 +56,14 @@
 }
 
 template  __host__ __device__ void hd2() { host_fn(); }
-// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ 
__device__ function}}
+// expected-error@-1 2 {{reference to __host__ function 'host_fn' in __host__ 
__device__ function}}
 __global__ void kernel() { hd2(); }
 
 __host__ __device__ void hd() { host_fn(); }
 // expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ 
__device__ function}}
 
 template  __host__ __device__ void hd3() { host_fn(); }
-// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ 
__device__ function}}
+// expected-error@-1 2 {{reference to __host__ function 'host_fn' in __host__ 
__device__ function}}
 __device__ void device_fn() { hd3(); }
 
 // No error because this is never instantiated.
Index: test/SemaCUDA/call-device-fn-from-host.cu
===
--- test/SemaCUDA/call-device-fn-from-host.cu
+++ test/SemaCUDA/call-device-fn-from-host.cu
@@ -37,7 +37,7 @@
 }
 
 template  __host__ __device__ void hd2() { device_fn(); }
-// expected-error@-1 {{reference to __device__ function 'device_fn' in 
__host__ __device__ function}}
+// expected-error@-1 2 {{reference to __device__ function 'device_fn' in 
__host__ __device__ function}}
 void host_fn() { hd2(); }
 
 __host__ __device__ void hd() { device_fn(); }
@@ -90,3 +90,8 @@
 static __host__ __device__ void hd_func() { device_fn(); }
 __global__ void kernel() { hd_func(); }
 void host_func(void) { kernel<<<1, 1>>>(); }
+
+// Should allow host function call kernel template with device function 
argument.
+__device__ void f();
+template __global__ void t() { F(); }
+__host__ void g() { t<<<1,1>>>(); }
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14760,6 +14760,9 @@
   if (FPT && isUnresolvedExceptionSpec(FPT->getExceptionSpecType()))
 ResolveExceptionSpec(Loc, FPT);
 
+  if (getLangOpts().CUDA)
+CheckCUDACall(Loc, Func);
+
   // If we don't need to mark the function as used, and we don't need to
   // try to provide a definition, there's nothing more to do.
   if ((Func->isUsed(/*CheckUsedAttr=*/false) || !OdrUse) &&
Index: lib/Sema/SemaCUDA.cpp
===
--- lib/Sema/SemaCUDA.cpp
+++ lib/Sema/SemaCUDA.cpp
@@ -675,6 +675,11 @@
 bool Sema::CheckCUDACall(SourceLocation Loc, FunctionDecl *Callee) {
   assert(getLangOpts().CUDA && "Should only be called during CUDA 
compilation");
   assert(Callee && "Callee may not be null.");
+
+  auto &ExprEvalCtx = ExprEvalContexts.back();
+  if (ExprEvalCtx.isUnevaluated() || ExprEvalCtx.isConstantEvaluated())
+return true;
+
   // FIXME: Is bailing out early correct here?  Should we instead assume that
   // the caller is a global initializer?
   FunctionDecl *Caller = dyn_cast(CurContext);


Index: test/SemaCUDA/call-host-fn-from-device.cu
===
--- test/SemaCUDA/call-host-fn-from-device.cu
+++ test/SemaCUDA/call-host-fn-from-device.cu
@@ -56,14 +56,14 @@
 }
 
 template  __host__ __device__ void hd2() { host_fn(); }
-// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
+// expected-error@-1 2 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
 __global__ void kernel() { hd2(); }
 
 __host__ __device__ void hd() { host_fn(); }
 // expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
 
 template  __host__ __device__ void hd3() { host_fn(); }
-// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
+// expected-error@-1 2 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
 __device__ void device_fn() { hd3(); }
 
 // No error because this is never instantiated.
Index: test/SemaCUDA/call-device-fn-from-host.cu
===
--- test/SemaCUDA/call-device-fn-from-host.cu
+++ test/SemaCUDA/call-device-fn-from-host.cu
@@ -37,7 +37,7 @@
 }
 
 template  __host__ __device__ void hd2() { device_fn(); }
-// expected-error@-1 {{reference to __device__ function 'device_fn' in __host__ __dev