[PATCH] D59296: [pp-trace] Delete -ignore and add generalized -callbacks

2019-03-17 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

@aaron.ballman Does the release note look good now? 


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59296



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

[PATCH] D59465: [analyzer] Add example plugin for checker option handling

2019-03-17 Thread Devin Coughlin via Phabricator via cfe-commits 
dcoughlin requested changes to this revision.
dcoughlin added a comment.
This revision now requires changes to proceed.

I'm not sure we really want to add any more examples of plugins.

Analyzer plugins aren't supported and likely never will be. We probably 
shouldn't give people false hope that a plugin model is going to be possible 
any time soon. I'm worried that this will just encourage people to shoot 
themselves in the foot.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59465



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

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

2019-03-17 Thread Devin Coughlin via Phabricator via cfe-commits 
dcoughlin added a comment.

This looks good to me. It is great to see this tested!

Did you consider separating the checker options from the non-checker options in 
the config dumper output? That would probably be easier to read.


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

https://reviews.llvm.org/D57922



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

[PATCH] D57860: [analyzer] Validate checker option names and values

2019-03-17 Thread Devin Coughlin via Phabricator via cfe-commits 
dcoughlin requested changes to this revision.
dcoughlin added a comment.
This revision now requires changes to proceed.

I think it would be better if the default for compatibility mode were 'true'. 
That way this change will be backwards compatible and clients who want to 
enforce stricter checking could enable it. Setting compatibility mode to be 
true in the driver is not sufficient since many build systems call the frontend 
directly.


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

https://reviews.llvm.org/D57860



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

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

2019-03-17 Thread Devin Coughlin via Phabricator via cfe-commits 
dcoughlin requested changes to this revision.
dcoughlin added a comment.
This revision now requires changes to proceed.

I'm pretty worried about exposing this flag to end users.

- Almost none of the options you've listed are user facing. Many represent 
options intended for use by static analyzer developers: debugging options, 
feature flags, and checkers that were never finished. Others represent 
mechanisms for build systems to control the behavior of the analyzer. Even 
these are not meant for end users to interact with but rather for implementers 
of build systems and IDEs. I don't think end users should have to understand 
these options to use the analyzer.
- The help text refers to analyzer implementation details (such as "SymRegion") 
that users won't have the context or knowledge to understand.
- The help text also recommends invoking -cc1 directly or through the driver 
with -Xclang. Neither of these are supported end-user interfaces to the 
analyzer. Instead, users should use scan-build or another tool (such as 
CodeChecker) that was designed to be used by humans.


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

https://reviews.llvm.org/D57858



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

r356354 - Add testcase from bug 41079

2019-03-17 Thread Matt Arsenault via cfe-commits 
Author: arsenm
Date: Sun Mar 17 16:16:31 2019
New Revision: 356354

URL: http://llvm.org/viewvc/llvm-project?rev=356354=rev
Log:
Add testcase from bug 41079

Modified:
cfe/trunk/test/CodeGen/builtin-expect.c

Modified: cfe/trunk/test/CodeGen/builtin-expect.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtin-expect.c?rev=356354=356353=356354=diff
==
--- cfe/trunk/test/CodeGen/builtin-expect.c (original)
+++ cfe/trunk/test/CodeGen/builtin-expect.c Sun Mar 17 16:16:31 2019
@@ -78,3 +78,20 @@ int switch_cond(int x) {
   return 0;
 }
 
+int variable_expected(int stuff) {
+// ALL-LABEL: define i32 @variable_expected(
+// O1: call i64 @llvm.expect.i64(i64 {{%.*}}, i64 {{%.*}})
+// O0-NOT: @llvm.expect
+
+  int res = 0;
+
+  switch (__builtin_expect(stuff, stuff)) {
+  case 0:
+res = 1;
+break;
+  default:
+break;
+  }
+
+  return res;
+}


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

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-17 Thread Reuben Thomas via Phabricator via cfe-commits 
reuk updated this revision to Diff 191050.

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

https://reviews.llvm.org/D55170

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9337,6 +9337,60 @@
   verifyFormat("T A::operator() ();", Space);
   verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
+  verifyFormat("int x = int (y);", Space);
+
+  FormatStyle SomeSpace = getLLVMStyle();
+  SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
+
+  verifyFormat("[]() -> float {}", SomeSpace);
+  verifyFormat("[] (auto foo) {}", SomeSpace);
+  verifyFormat("[foo]() -> int {}", SomeSpace);
+  verifyFormat("int f();", SomeSpace);
+  verifyFormat("void f (int a, T b) {\n"
+   "  while (true)\n"
+   "continue;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SomeSpace);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SomeSpace);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("A::A() : a (1) {}", SomeSpace);
+  verifyFormat("void f() __attribute__ ((asdf));", SomeSpace);
+  verifyFormat("*( + 1);\n"
+   "&(()[1]);\n"
+   "a[(b + c) * d];\n"
+   "(((a + 1) * 2) + 3) * 4;",
+   SomeSpace);
+  verifyFormat("#define A(x) x", SomeSpace);
+  verifyFormat("#define A (x) x", SomeSpace);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SomeSpace);
+  verifyFormat("auto i = std::make_unique (5);", SomeSpace);
+  verifyFormat("size_t x = sizeof (x);", SomeSpace);
+  verifyFormat("auto f (int x) -> decltype (x);", SomeSpace);
+  verifyFormat("int f (T x) noexcept (x.create());", SomeSpace);
+  verifyFormat("alignas (128) char a[128];", SomeSpace);
+  verifyFormat("size_t x = alignof (MyType);", SomeSpace);
+  verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");",
+   SomeSpace);
+  verifyFormat("int f() throw (Deprecated);", SomeSpace);
+  verifyFormat("typedef void (*cb) (int);", SomeSpace);
+  verifyFormat("T A::operator()();", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("int x = int (y);", SomeSpace);
+  verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
@@ -11121,6 +11175,8 @@
   FormatStyle::SBPO_Always);
   CHECK_PARSE("SpaceBeforeParens: ControlStatements", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
+  CHECK_PARSE("SpaceBeforeParens: NonEmptyParentheses", SpaceBeforeParens,
+  FormatStyle::SBPO_NonEmptyParentheses);
   // For backward compatibility:
   CHECK_PARSE("SpaceAfterControlStatementKeyword: false", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -164,6 +164,8 @@
   unsigned splitPenalty(const AnnotatedLine , const FormatToken ,
 bool InFunctionDecl);
 
+  bool spaceRequiredBeforeParens(const FormatToken ) const;
+
   bool spaceRequiredBetween(const AnnotatedLine , const FormatToken ,
 const FormatToken );
 
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2400,6 +2400,12 @@
   return 3;
 }
 
+bool TokenAnnotator::spaceRequiredBeforeParens(const FormatToken ) const {
+  return Style.SpaceBeforeParens == FormatStyle::SBPO_Always ||
+ (Style.SpaceBeforeParens == FormatStyle::SBPO_NonEmptyParentheses &&
+  Right.ParameterCount > 0);
+}
+
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine ,
   const FormatToken ,
   const FormatToken ) {
@@ -2539,19 +2545,19 @@
   return true;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
-(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
-  tok::kw_switch, tok::kw_case, TT_ForEachMacro,
-  TT_ObjCForIn) ||
- Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
-

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-17 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added inline comments.



Comment at: include/clang/Format/Format.h:1578
   /// \endcode
   bool SpaceBeforeCpp11BracedList;
 

boolean here not enum see comment below



Comment at: lib/Format/TokenAnnotator.cpp:2608
+(Style.SpaceBeforeCpp11BracedList == FormatStyle::SBBLO_Always ||
+ (Style.SpaceBeforeCpp11BracedList == FormatStyle::SBBLO_NonEmpty &&
+  Right.ParameterCount > 0)))

Did the patch upload correctly? this is still not showing a bool for 
SpaceBeforeCpp11BrackedList


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

https://reviews.llvm.org/D55170



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-17 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

In D59467#1432675 , @Tyker wrote:

> if likely/unlikely can be applied to any statement or label what should be 
> generated when it is applied to a statement that don't have any conditional 
> branch ? should it be ignored without warning or error ?


Disregard previous [deleted] comment from me.
Yes, i believe it should simply be always accepted, for any 
`isa()`.
How to model that in codegen i'm not sure yet. Likely by affecting the nearest 
(up the AST) branch / switch / etc.


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-17 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

In D59467#1432675 , @Tyker wrote:

> if likely/unlikely can be applied to any statement or label what should be 
> generated when it is applied to a statement that don't have any conditional 
> branch ? should it be ignored without warning or error ?


I think this should be modelled as a new statement type, `AttributeDeclStmt` or 
even `LikelihoodDeclStmt`.
I'm not fully sure how to model it in codegen.


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-17 Thread Gauthier via Phabricator via cfe-commits 
Tyker added a comment.

if likely/unlikely can be applied to any statement or label what should be 
generated when it is applied to a statement that don't have any conditional 
branch ? should it be ignored without warning or error ?


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-17 Thread Gauthier via Phabricator via cfe-commits 
Tyker marked an inline comment as done.
Tyker added a comment.

I added the sema testes


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-17 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1151
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201903>, CXX11<"clang", "likely">];
+  let Documentation = [LikelyDocs];

I think this should have a C spelling as well, so I'd probably go with: 
`[CXX11<"", "likely", 201903L>, Clang<"likely">]` and then similar for 
`unlikely`. I would probably use the same semantic attribute for both (so add 
both spellings and an accessor to differentiate). Rather than naming the 
attribute `Likely`, I would go with `Likelihood` so it covers both spellings 
more naturally.



Comment at: clang/include/clang/Basic/Attr.td:1153
+  let Documentation = [LikelyDocs];
+}
+

Can you add a commented-out `Subjects` that take `Stmt` and `LabelStmt`? It's 
not useful now, but I still hope that someday we can tablegen more checks for 
statement and type attributes like we already do for declarations.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8164
+def err_likely_attr_invalid_placement : Error<
+  "likely annotation can't apear here">;
+

lebedev.ri wrote:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > appear
> > Please mark fixed comments as fixed (checkbox).
> 'here'?
> Would be nicer to be more explanatory.
> `likely annotation can't appear on %0; can only appear on x, y, x`
This should follow the pattern used by decl attributes:
```
def err_attribute_wrong_stmt_type : Error<
  "%0 attribute only applies to statements and labels">;
```
There may be a diagnostic used for `[[fallthrough]]` that could be combined 
with this diagnostic, since that attribute can only be applied to null 
statements.


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-17 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

I believe this has the incorrect modeling -- in the following code example, the 
attribute appertains to the substatement of the if statement, not the if 
statement itself:

  if (foo) [[likely]] {
blah;
  }

This falls out from:  http://eel.is/c++draft/stmt.stmt#1. You model this by 
applying the attribute to the `if` statement, which will have the wrong 
semantics when you hook up the hints to the backend.




Comment at: clang/test/SemaCXX/cxx2a-likely-attr.cpp:27
+  switch (n) {
+[[likely]] return; // expected-error {{likely annotation can't appear 
here}}
+

Why? The attribute appertains to statements, and `return` is a statement. This 
should be accepted, I believe.



Comment at: clang/test/SemaCXX/cxx2a-likely-attr.cpp:29
+
+  case 0:
+if (1) [[likely, likely]] // expected-error {{attribute 'likely' cannot 
appear multiple times in an attribute specifier}}

Missing a test case that the attribute should apply to labels like this (should 
also have a test for non-switch labels as well).



Comment at: clang/test/SemaCXX/cxx2a-likely-attr.cpp:30
+  case 0:
+if (1) [[likely, likely]] // expected-error {{attribute 'likely' cannot 
appear multiple times in an attribute specifier}}
+  return ;

Also missing the test case covering 
http://eel.is/c++draft/dcl.attr.likelihood#1.sentence-3


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

https://reviews.llvm.org/D59467



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

[PATCH] D59407: [clangd] Add RelationSlab

2019-03-17 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge updated this revision to Diff 191036.
nridge marked an inline comment as done.
nridge added a comment.
Herald added a subscriber: mgorny.

Address review comments, except for the deduplication which is still under 
discussion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/Relation.cpp
  clang-tools-extra/clangd/index/Relation.h

Index: clang-tools-extra/clangd/index/Relation.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/Relation.h
@@ -0,0 +1,124 @@
+//===--- Ref.h ---*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RELATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RELATION_H
+
+#include "SymbolID.h"
+#include "SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/Support/StringSaver.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+struct RelationKey {
+  SymbolID Symbol;
+  index::SymbolRole Kind;
+
+  bool operator==(const RelationKey ) const {
+return Symbol == Other.Symbol && Kind == Other.Kind;
+  }
+
+private:
+  friend llvm::hash_code hash_value(const RelationKey ) {
+return llvm::hash_combine(Key.Symbol, static_cast(Key.Kind));
+  }
+};
+
+class RelationSlab {
+public:
+  // Key.Symbol is Key.Kind of every symbol in Value.
+  // For example, if Key.Kind == SymbolRole::RelationChildOf,
+  // then Key.Symbol is the child of every symbol in Value (i.e. the symbols
+  // in Value are the base classes of Key.Symbol).
+  struct Relation {
+RelationKey Key;
+llvm::ArrayRef Value;
+  };
+  using value_type = std::pair>;
+  using const_iterator = std::vector::const_iterator;
+  using iterator = const_iterator;
+
+  RelationSlab() = default;
+  RelationSlab(RelationSlab &) = default;
+  RelationSlab =(RelationSlab &) = default;
+
+  const_iterator begin() const { return Relations.begin(); }
+  const_iterator end() const { return Relations.end(); }
+  size_t size() const { return Relations.size(); }
+  size_t numRelations() const { return NumRelations; }
+  bool empty() const { return Relations.empty(); }
+
+  size_t bytes() const {
+return sizeof(*this) + Arena.getTotalMemory() +
+   sizeof(value_type) * Relations.capacity();
+  }
+
+  // RelationSlab::Builder is a mutable container that can 'freeze' to
+  // RelationSlab.
+  class Builder {
+  public:
+Builder() {}
+// Adds a relation to the slab.
+void insert(const RelationKey , const SymbolID );
+// Consumes the builder to finalize the slab.
+RelationSlab build() &&;
+
+  private:
+llvm::BumpPtrAllocator Arena;
+llvm::DenseMap> Relations;
+  };
+
+private:
+  RelationSlab(std::vector Relations, llvm::BumpPtrAllocator Arena,
+   size_t NumRelations)
+  : Arena(std::move(Arena)), Relations(std::move(Relations)),
+NumRelations(NumRelations) {}
+
+  llvm::BumpPtrAllocator Arena;
+  std::vector Relations;
+  // Number of all relations.
+  size_t NumRelations = 0;
+};
+
+} // namespace clangd
+} // namespace clang
+
+namespace llvm {
+
+// Support RelationKeys as DenseMap keys.
+template <> struct DenseMapInfo {
+  static inline clang::clangd::RelationKey getEmptyKey() {
+return {DenseMapInfo::getEmptyKey(),
+clang::index::SymbolRole::RelationChildOf};
+  }
+
+  static inline clang::clangd::RelationKey getTombstoneKey() {
+return {DenseMapInfo::getTombstoneKey(),
+clang::index::SymbolRole::RelationChildOf};
+  }
+
+  static unsigned getHashValue(const clang::clangd::RelationKey ) {
+return hash_value(Key);
+  }
+
+  static bool isEqual(const clang::clangd::RelationKey ,
+  const clang::clangd::RelationKey ) {
+return LHS == RHS;
+  }
+};
+
+} // namespace llvm
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RELATION_H
Index: clang-tools-extra/clangd/index/Relation.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/index/Relation.cpp
@@ -0,0 +1,35 @@
+//===--- Ref.cpp -*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//

[PATCH] D59407: [clangd] Add RelationSlab

2019-03-17 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge marked 7 inline comments as done.
nridge added inline comments.



Comment at: clang-tools-extra/clangd/index/Index.h:43
+public:
+  using value_type = std::pair>;
+  using const_iterator = std::vector::const_iterator;

kadircet wrote:
> gribozavr wrote:
> > `struct Relation`?  And in the comments for it, please explain which way 
> > the relationship is directed (is the SymbolID in the key the subtype?  or 
> > is the SymbolID in the ArrayRef the subtype?).
> Ah exactly my thoughts, forget to mention this.
> 
> I believe current usage is the counter-intuitive one. For example, we will 
> most likely query with something like:
> `getRelations(SymbolID, baseOf)` to get all relations where `SymbolID` is 
> `baseOf` something else(which says get children of `SymbolID`)
> 
> So that this valueType can be read like, 
> ```
> `SymbolID` is `RelationKind` every `SymbolID inside array`
> ```
> WDYT?
The way I was thinking of it is that `getRelations(SymbolID, baseOf)` would 
return all the bases of `SymbolID`. However, the opposite interpretation is 
also reasonable, we just need to pick one and document it. I'm happy to go with 
your suggested one.



Comment at: clang-tools-extra/clangd/index/Index.h:59
+return sizeof(*this) + Arena.getTotalMemory() +
+   sizeof(value_type) * Relations.size();
+  }

kadircet wrote:
> use capacity instead of size
Note, `RefSlab::bytes()` (which I where I copied this from) uses `size()` as 
well. Should I change that too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-17 Thread Gauthier via Phabricator via cfe-commits 
Tyker updated this revision to Diff 191035.

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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/SemaCXX/cxx2a-likely-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likely-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likely-attr.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++2a
+
+int f(int i) {
+  if (i == 1)
+  {
+  return 0;
+}
+  else if (i == 2) [[likely]]
+return 1;
+  return 3;
+}
+
+[[likely]] typedef int n1; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2; // expected-error {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]]; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E { // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  One
+};
+
+[[likely]] // expected-error {{'likely' attribute cannot be applied to a declaration}}
+void g(void) {
+  [[likely]] int n; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  [[likely]] ++n; // expected-error {{likely annotation can't appear here}}
+  
+  switch (n) {
+[[likely]] return; // expected-error {{likely annotation can't appear here}}
+
+  case 0:
+if (1) [[likely, likely]] // expected-error {{attribute 'likely' cannot appear multiple times in an attribute specifier}}
+  return ;
+if (1) [[likely(0)]] // expected-error {{attribute 'likely' cannot have an argument list}}
+  return ;
+break;
+  }
+}
+
Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -23,6 +23,21 @@
 // CHECK-NEXT:   FallThroughAttr
 // CHECK-NEXT:   NullStmt
 
+int TestLikelyAttribute(int i) {
+  if (i == 1) [[likely]] {
+return 0;
+  } else if (i == 2) [[likely]]
+return 1;
+  return 2;
+}
+// CHECK:  FunctionDecl{{.*}}TestLikelyAttribute
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: IfStmt
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: IfStmt
+
 [[clang::warn_unused_result]] int TestCXX11DeclAttr();
 // CHECK:  FunctionDecl{{.*}}TestCXX11DeclAttr
 // CHECK-NEXT:   WarnUnusedResultAttr
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,21 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelyAttr(Sema , Stmt *St, const ParsedAttr ,
+   SourceRange Range) {
+  LikelyAttr Attr(A.getRange(), S.Context,
+   A.getAttributeSpellingListIndex());
+
+  if (!llvm::isa(St)) {
+S.Diag(A.getLoc(), diag::err_likely_attr_invalid_placement) << A.getName();
+  }
+
+  if (!S.getLangOpts().CPlusPlus2a && !A.getScopeName())
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  return ::new (S.Context) auto(Attr);
+}
+
 static Attr *handleSuppressAttr(Sema , Stmt *St, const ParsedAttr ,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -336,6 +351,8 @@
 return nullptr;
   case ParsedAttr::AT_FallThrough:
 return handleFallThroughAttr(S, St, A, Range);
+  case ParsedAttr::AT_Likely:
+return handleLikelyAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
 return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1216,6 +1216,9 @@
 : Sema::ConditionKind::Boolean))
 return StmtError();
 
+  ParsedAttributesWithRange Attrs(AttrFactory);
+  MaybeParseCXX11Attributes(Attrs);
+
   llvm::Optional ConstexprCondition;
   if (IsConstexpr)
 ConstexprCondition = Cond.getKnownValue();
@@ -1314,8 +1317,13 @@
   if (ElseStmt.isInvalid())
 ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
 
-  return Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
- ThenStmt.get(), ElseLoc, ElseStmt.get());
+  StmtResult Stmt = Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
+ ThenStmt.get(), ElseLoc, ElseStmt.get());
+
+  if (!Attrs.empty())
+return Actions.ProcessStmtAttributes(Stmt.get(), Attrs, Attrs.Range);
+
+  return Stmt;
 }
 
 /// ParseSwitchStatement
Index:

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-17 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

In D59467#1432587 , @Tyker wrote:

> i added tests as you requested. i didn't add test for the codegen as it 
> wasn't affected.


Ah right, there wasn't some other clang-specific spelling for this already it 
seems, so indeed codegen needs wiring up too.

Sema tests still missing. (correct usage of attribute in various places with 
different `-std=?` params, incorrect usage of attribute in incorrect places, 
etc)




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8164
+def err_likely_attr_invalid_placement : Error<
+  "likely annotation can't apear here">;
+

lebedev.ri wrote:
> appear
Please mark fixed comments as fixed (checkbox).



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8164
+def err_likely_attr_invalid_placement : Error<
+  "likely annotation can't apear here">;
+

lebedev.ri wrote:
> lebedev.ri wrote:
> > appear
> Please mark fixed comments as fixed (checkbox).
'here'?
Would be nicer to be more explanatory.
`likely annotation can't appear on %0; can only appear on x, y, x`



Comment at: clang/lib/Sema/SemaStmtAttr.cpp:59
+
+  if (!llvm::isa(St)) {
+S.Diag(A.getLoc(), diag::err_likely_attr_invalid_placement) << A.getName();

Where else can it appear?


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-17 Thread Gauthier via Phabricator via cfe-commits 
Tyker updated this revision to Diff 191034.

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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-dump-attr.cpp

Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -23,6 +23,21 @@
 // CHECK-NEXT:   FallThroughAttr
 // CHECK-NEXT:   NullStmt
 
+int TestLikelyAttribute(int i) {
+  if (i == 1) [[likely]] {
+return 0;
+  } else if (i == 2) [[likely]]
+return 1;
+  return 2;
+}
+// CHECK:  FunctionDecl{{.*}}TestLikelyAttribute
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: IfStmt
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: IfStmt
+
 [[clang::warn_unused_result]] int TestCXX11DeclAttr();
 // CHECK:  FunctionDecl{{.*}}TestCXX11DeclAttr
 // CHECK-NEXT:   WarnUnusedResultAttr
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,21 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelyAttr(Sema , Stmt *St, const ParsedAttr ,
+   SourceRange Range) {
+  LikelyAttr Attr(A.getRange(), S.Context,
+   A.getAttributeSpellingListIndex());
+
+  if (!llvm::isa(St)) {
+S.Diag(A.getLoc(), diag::err_likely_attr_invalid_placement) << A.getName();
+  }
+
+  if (!S.getLangOpts().CPlusPlus2a && !A.getScopeName())
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  return ::new (S.Context) auto(Attr);
+}
+
 static Attr *handleSuppressAttr(Sema , Stmt *St, const ParsedAttr ,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -336,6 +351,8 @@
 return nullptr;
   case ParsedAttr::AT_FallThrough:
 return handleFallThroughAttr(S, St, A, Range);
+  case ParsedAttr::AT_Likely:
+return handleLikelyAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
 return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1216,6 +1216,9 @@
 : Sema::ConditionKind::Boolean))
 return StmtError();
 
+  ParsedAttributesWithRange Attrs(AttrFactory);
+  MaybeParseCXX11Attributes(Attrs);
+
   llvm::Optional ConstexprCondition;
   if (IsConstexpr)
 ConstexprCondition = Cond.getKnownValue();
@@ -1314,8 +1317,13 @@
   if (ElseStmt.isInvalid())
 ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
 
-  return Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
- ThenStmt.get(), ElseLoc, ElseStmt.get());
+  StmtResult Stmt = Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
+ ThenStmt.get(), ElseLoc, ElseStmt.get());
+
+  if (!Attrs.empty())
+return Actions.ProcessStmtAttributes(Stmt.get(), Attrs, Attrs.Range);
+
+  return Stmt;
 }
 
 /// ParseSwitchStatement
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -3858,6 +3858,7 @@
   case ParsedAttr::AT_Deprecated:
   case ParsedAttr::AT_FallThrough:
   case ParsedAttr::AT_CXX11NoReturn:
+  case ParsedAttr::AT_Likely:
 return true;
   case ParsedAttr::AT_WarnUnusedResult:
 return !ScopeName && AttrName->getName().equals("nodiscard");
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7339,6 +7339,8 @@
   "use of the %0 attribute is a C++14 extension">, InGroup;
 def ext_cxx17_attr : Extension<
   "use of the %0 attribute is a C++17 extension">, InGroup;
+def ext_cxx2a_attr : Extension<
+  "use of the %0 attribute is a C++2a extension">, InGroup;
 
 def warn_unused_comparison : Warning<
   "%select{equality|inequality|relational|three-way}0 comparison result unused">,
@@ -8158,6 +8160,9 @@
   "fallthrough annotation in unreachable code">,
   InGroup, DefaultIgnore;
 
+def err_likely_attr_invalid_placement : Error<
+  "likely annotation can't appear here">;
+
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
   InGroup, DefaultIgnore;
Index: clang/include/clang/Basic/AttrDocs.td
===

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-17 Thread Gauthier via Phabricator via cfe-commits 
Tyker updated this revision to Diff 191031.
Tyker added a comment.

i added tests as you requested. i didn't add test for the codegen as it wasn't 
affected.


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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-dump-attr.cpp

Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -23,6 +23,21 @@
 // CHECK-NEXT:   FallThroughAttr
 // CHECK-NEXT:   NullStmt
 
+int TestLikelyAttribute(int i) {
+  if (i == 1) [[likely]] {
+return 0;
+  } else if (i == 2) [[likely]]
+return 1;
+  return 2;
+}
+// CHECK:  FunctionDecl{{.*}}TestLikelyAttribute
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: IfStmt
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: IfStmt
+
 [[clang::warn_unused_result]] int TestCXX11DeclAttr();
 // CHECK:  FunctionDecl{{.*}}TestCXX11DeclAttr
 // CHECK-NEXT:   WarnUnusedResultAttr
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,21 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelyAttr(Sema , Stmt *St, const ParsedAttr ,
+   SourceRange Range) {
+  LikelyAttr Attr(A.getRange(), S.Context,
+   A.getAttributeSpellingListIndex());
+
+  if (!llvm::isa(St)) {
+S.Diag(A.getLoc(), diag::err_likely_attr_invalid_placement) << A.getName();
+  }
+
+  if (!S.getLangOpts().CPlusPlus2a && !A.getScopeName())
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  return ::new (S.Context) auto(Attr);
+}
+
 static Attr *handleSuppressAttr(Sema , Stmt *St, const ParsedAttr ,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -336,6 +351,8 @@
 return nullptr;
   case ParsedAttr::AT_FallThrough:
 return handleFallThroughAttr(S, St, A, Range);
+  case ParsedAttr::AT_Likely:
+return handleLikelyAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
 return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1216,6 +1216,9 @@
 : Sema::ConditionKind::Boolean))
 return StmtError();
 
+  ParsedAttributesWithRange Attrs(AttrFactory);
+  MaybeParseCXX11Attributes(Attrs);
+
   llvm::Optional ConstexprCondition;
   if (IsConstexpr)
 ConstexprCondition = Cond.getKnownValue();
@@ -1314,8 +1317,13 @@
   if (ElseStmt.isInvalid())
 ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
 
-  return Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
- ThenStmt.get(), ElseLoc, ElseStmt.get());
+  StmtResult Stmt = Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
+ ThenStmt.get(), ElseLoc, ElseStmt.get());
+
+  if (!Attrs.empty())
+return Actions.ProcessStmtAttributes(Stmt.get(), Attrs, Attrs.Range);
+
+  return Stmt;
 }
 
 /// ParseSwitchStatement
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -3858,6 +3858,7 @@
   case ParsedAttr::AT_Deprecated:
   case ParsedAttr::AT_FallThrough:
   case ParsedAttr::AT_CXX11NoReturn:
+  case ParsedAttr::AT_Likely:
 return true;
   case ParsedAttr::AT_WarnUnusedResult:
 return !ScopeName && AttrName->getName().equals("nodiscard");
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7339,6 +7339,8 @@
   "use of the %0 attribute is a C++14 extension">, InGroup;
 def ext_cxx17_attr : Extension<
   "use of the %0 attribute is a C++17 extension">, InGroup;
+def ext_cxx2a_attr : Extension<
+  "use of the %0 attribute is a C++2a extension">, InGroup;
 
 def warn_unused_comparison : Warning<
   "%select{equality|inequality|relational|three-way}0 comparison result unused">,
@@ -8158,6 +8160,9 @@
   "fallthrough annotation in unreachable code">,
   InGroup, DefaultIgnore;
 
+def err_likely_attr_invalid_placement : Error<
+  "likely annotation can't appear here">;
+
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
   InGroup, DefaultIgnore;

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

2019-03-17 Thread 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 code generation for OpenMP 5.0 declare mapper 
(user-defined mapper) constructs. For each declare mapper, a synchronous 
mapping function as well as an asynchronous one are generated, which will be 
called by the runtime to achieve user defined mapping.


Repository:
  rC Clang

https://reviews.llvm.org/D59474

Files:
  include/clang/AST/GlobalDecl.h
  lib/AST/ASTContext.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/ModuleBuilder.cpp
  test/OpenMP/declare_mapper_codegen.cpp

Index: test/OpenMP/declare_mapper_codegen.cpp
===
--- test/OpenMP/declare_mapper_codegen.cpp
+++ test/OpenMP/declare_mapper_codegen.cpp
@@ -1,92 +1,770 @@
-///==///
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap %s
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap %s
-
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER
 
+///==///
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix CK0 --check-prefix CK0-64 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -femit-all-decls -disable-llvm-passes -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix CK0 --check-prefix CK0-64 %s
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix CK0 --check-prefix CK0-32 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -femit-all-decls -disable-llvm-passes -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix CK0 --check-prefix CK0-32 %s
+
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple

[PATCH] D59407: [clangd] Add RelationSlab

2019-03-17 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge added a comment.

In D59407#1430656 , @kadircet wrote:

> I believe it makes sense to deduplicate SymbolIDs for RelationSlab.
>  Up until now, we mostly had only one occurence of a SymbolID in a Slab, but 
> RelationSlab does not follow that assumption.


Just to make sure I understand, do you mean:

(A) When adding a `SymbolID` to an entry's value, check that it's not already 
there; or
(B) Try to conserve space by not storing `SymbolID`s directly in the entries, 
but storing an index into a separate list of unique `SymbolID`s.

If it's (B), how many bytes should the index be? Are the space gains worth the 
complexity, given that `SymbolID` is only 8 bytes to begin with? (As compared 
to say, the filenames in `Ref`, which can be much longer, making this sort of 
optimization more clearly worth it.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D58880: [WIP] [Looking for API feedback] [clangd] Type hierarchy subtypes

2019-03-17 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge added a comment.

In D58880#1422494 , @kadircet wrote:

> Also please have a look at D59083 , and make 
> sure it helps implement what you have in might and let me know if there's 
> anything missing.


I verified that with D59083  + D59354 
, I'm able to get my template specialization 
tests passing. Thanks for working on those!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58880



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

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

2019-03-17 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge added a comment.

Great, thanks for the reviews!

Could you commit the patch as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370



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

[PATCH] D59318: [clang-tidy] add an overload for diag method that allows users to provide a diagnostic name rather than using the check name when building a diagnostic.

2019-03-17 Thread Harshal Lehri via Phabricator via cfe-commits 
htl abandoned this revision.
htl added a comment.

In D59318#1430829 , @JonasToth wrote:

> What is the reason you want this change to happen? I think this gives the 
> chance to create inconsistencies which we should avoid.


Sorry this was a bit rushed. I was trying to pass some metadata in the 
diagnostic name so that another tool that uses the diagnostic name can perform 
some conditional evaluation based on the data. I will try coming up with a 
better plan and design for such a change.

Thank you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59318



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

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

2019-03-17 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: include/clang/AST/ASTTypeTraits.h:81
 
+  /// \{
+  /// Return the AST node kind of this ASTNodeKind.

lebedev.ri wrote:
> aaron.ballman wrote:
> > lebedev.ri wrote:
> > > aaron.ballman wrote:
> > > > These markings are a bit strange, can you explain them to me?
> > > It is weird, but i think this is the right solution.
> > > See `isAllowedToContainClause()` narrower.
> > > This `asOMPClauseKind()` allows to pass a whole matcher, and then distill 
> > > the inner output node type.
> > > I.e. now i can spell 
> > > `ompExecutableDirective(isAllowedToContainClause(ompDefaultClause()))`
> > > (and the `ompDefaultClause()` won't actually be used for matching!), 
> > > instead of doing something like e.g.
> > > `ompExecutableDirective(isAllowedToContainClause(OMPC_default))`
> > > which looks horrible, and will likely not work well with `clang-query`?
> > I think we may be talking about different things. I only meant the `\{` and 
> > `\}` comment markers. :-D I think the design is reasonable enough, but I'm 
> > not an OpenMP expert. Truth be told, I was mostly surprised that these 
> > pragmas will have corresponding AST matchers. I thought they were 
> > preprocessor directives and thus would be handled at that level, rather 
> > than the AST level. My only concern about the design of this is a pretty 
> > minor one: what happens if someone is using preprocessor callbacks and AST 
> > matchers at the same time -- will they get duplicate results for OpenMP 
> > directives? I suspect they will, but that doesn't mean we shouldn't AST 
> > match OpenMP clauses (they are in the AST, after all).
> Oh duh xD
> I just copied them from around the `getFromNode()` up there ^.
> I believe they signify that these functions should be displayed as a group in 
> doxygen.
> 
> I don't know what will happen when using preprocessor callbacks and AST 
> matchers at the same time,
> but i //think// that would be the issue of the user in question.
> I just copied them from around the getFromNode() up there ^.
> I believe they signify that these functions should be displayed as a group in 
> doxygen.

I thought so as well, but there's no grouping here. I'd probably drop them 
until it's needed.

> I don't know what will happen when using preprocessor callbacks and AST 
> matchers at the same time, but i think that would be the issue of the user in 
> question.

I agree.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57112



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

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

2019-03-17 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri marked 3 inline comments as done.
lebedev.ri added inline comments.



Comment at: include/clang/AST/ASTTypeTraits.h:81
 
+  /// \{
+  /// Return the AST node kind of this ASTNodeKind.

aaron.ballman wrote:
> lebedev.ri wrote:
> > aaron.ballman wrote:
> > > These markings are a bit strange, can you explain them to me?
> > It is weird, but i think this is the right solution.
> > See `isAllowedToContainClause()` narrower.
> > This `asOMPClauseKind()` allows to pass a whole matcher, and then distill 
> > the inner output node type.
> > I.e. now i can spell 
> > `ompExecutableDirective(isAllowedToContainClause(ompDefaultClause()))`
> > (and the `ompDefaultClause()` won't actually be used for matching!), 
> > instead of doing something like e.g.
> > `ompExecutableDirective(isAllowedToContainClause(OMPC_default))`
> > which looks horrible, and will likely not work well with `clang-query`?
> I think we may be talking about different things. I only meant the `\{` and 
> `\}` comment markers. :-D I think the design is reasonable enough, but I'm 
> not an OpenMP expert. Truth be told, I was mostly surprised that these 
> pragmas will have corresponding AST matchers. I thought they were 
> preprocessor directives and thus would be handled at that level, rather than 
> the AST level. My only concern about the design of this is a pretty minor 
> one: what happens if someone is using preprocessor callbacks and AST matchers 
> at the same time -- will they get duplicate results for OpenMP directives? I 
> suspect they will, but that doesn't mean we shouldn't AST match OpenMP 
> clauses (they are in the AST, after all).
Oh duh xD
I just copied them from around the `getFromNode()` up there ^.
I believe they signify that these functions should be displayed as a group in 
doxygen.

I don't know what will happen when using preprocessor callbacks and AST 
matchers at the same time,
but i //think// that would be the issue of the user in question.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57112



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

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

2019-03-17 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: include/clang/AST/ASTTypeTraits.h:81
 
+  /// \{
+  /// Return the AST node kind of this ASTNodeKind.

lebedev.ri wrote:
> aaron.ballman wrote:
> > These markings are a bit strange, can you explain them to me?
> It is weird, but i think this is the right solution.
> See `isAllowedToContainClause()` narrower.
> This `asOMPClauseKind()` allows to pass a whole matcher, and then distill the 
> inner output node type.
> I.e. now i can spell 
> `ompExecutableDirective(isAllowedToContainClause(ompDefaultClause()))`
> (and the `ompDefaultClause()` won't actually be used for matching!), instead 
> of doing something like e.g.
> `ompExecutableDirective(isAllowedToContainClause(OMPC_default))`
> which looks horrible, and will likely not work well with `clang-query`?
I think we may be talking about different things. I only meant the `\{` and 
`\}` comment markers. :-D I think the design is reasonable enough, but I'm not 
an OpenMP expert. Truth be told, I was mostly surprised that these pragmas will 
have corresponding AST matchers. I thought they were preprocessor directives 
and thus would be handled at that level, rather than the AST level. My only 
concern about the design of this is a pretty minor one: what happens if someone 
is using preprocessor callbacks and AST matchers at the same time -- will they 
get duplicate results for OpenMP directives? I suspect they will, but that 
doesn't mean we shouldn't AST match OpenMP clauses (they are in the AST, after 
all).


Repository:
  rC Clang

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

https://reviews.llvm.org/D57112



___
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

2019-03-17 Thread Tolga Mizrak via Phabricator via cfe-commits 
to-miz added a comment.

I don't have commit access. It would be nice if someone could commit it in my 
place.


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] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-17 Thread Reuben Thomas via Phabricator via cfe-commits 
reuk updated this revision to Diff 191015.
reuk added a comment.

@MyDeveloperDay I'm sorry, you're absolutely correct. I'd got some other 
JUCE-related changes mixed up in this PR. Should be fixed now.


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

https://reviews.llvm.org/D55170

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9337,6 +9337,60 @@
   verifyFormat("T A::operator() ();", Space);
   verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
+  verifyFormat("int x = int (y);", Space);
+
+  FormatStyle SomeSpace = getLLVMStyle();
+  SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
+
+  verifyFormat("[]() -> float {}", SomeSpace);
+  verifyFormat("[] (auto foo) {}", SomeSpace);
+  verifyFormat("[foo]() -> int {}", SomeSpace);
+  verifyFormat("int f();", SomeSpace);
+  verifyFormat("void f (int a, T b) {\n"
+   "  while (true)\n"
+   "continue;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SomeSpace);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SomeSpace);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("A::A() : a (1) {}", SomeSpace);
+  verifyFormat("void f() __attribute__ ((asdf));", SomeSpace);
+  verifyFormat("*( + 1);\n"
+   "&(()[1]);\n"
+   "a[(b + c) * d];\n"
+   "(((a + 1) * 2) + 3) * 4;",
+   SomeSpace);
+  verifyFormat("#define A(x) x", SomeSpace);
+  verifyFormat("#define A (x) x", SomeSpace);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SomeSpace);
+  verifyFormat("auto i = std::make_unique (5);", SomeSpace);
+  verifyFormat("size_t x = sizeof (x);", SomeSpace);
+  verifyFormat("auto f (int x) -> decltype (x);", SomeSpace);
+  verifyFormat("int f (T x) noexcept (x.create());", SomeSpace);
+  verifyFormat("alignas (128) char a[128];", SomeSpace);
+  verifyFormat("size_t x = alignof (MyType);", SomeSpace);
+  verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");",
+   SomeSpace);
+  verifyFormat("int f() throw (Deprecated);", SomeSpace);
+  verifyFormat("typedef void (*cb) (int);", SomeSpace);
+  verifyFormat("T A::operator()();", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("int x = int (y);", SomeSpace);
+  verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
@@ -11121,6 +11175,8 @@
   FormatStyle::SBPO_Always);
   CHECK_PARSE("SpaceBeforeParens: ControlStatements", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
+  CHECK_PARSE("SpaceBeforeParens: NonEmptyParentheses", SpaceBeforeParens,
+  FormatStyle::SBPO_NonEmptyParentheses);
   // For backward compatibility:
   CHECK_PARSE("SpaceAfterControlStatementKeyword: false", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -164,6 +164,8 @@
   unsigned splitPenalty(const AnnotatedLine , const FormatToken ,
 bool InFunctionDecl);
 
+  bool spaceRequiredBeforeParens(const FormatToken ) const;
+
   bool spaceRequiredBetween(const AnnotatedLine , const FormatToken ,
 const FormatToken );
 
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2400,6 +2400,12 @@
   return 3;
 }
 
+bool TokenAnnotator::spaceRequiredBeforeParens(const FormatToken ) const {
+  return Style.SpaceBeforeParens == FormatStyle::SBPO_Always ||
+ (Style.SpaceBeforeParens == FormatStyle::SBPO_NonEmptyParentheses &&
+  Right.ParameterCount > 0);
+}
+
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine ,
   const FormatToken ,
   const FormatToken ) {
@@ -2539,19 +2545,19 @@
   return true;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
-(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
-  tok::kw_switch,

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

2019-03-17 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added inline comments.



Comment at: unittests/clang-tidy/AddConstTest.cpp:733
+  StringRef T = "template  void f(T v) \n";
+  StringRef S = "{ T target = v; }";
+  auto Cat = [](StringRef S) { return (T + S).str(); };

lebedev.ri wrote:
> JonasToth wrote:
> > alexfh wrote:
> > > It would be interesting to see test cases with multiple instantiations of 
> > > the template the fix applies to.
> > I added test for a template function with many instantiations, but there 
> > should not be a difference between the instantiations, because only the 
> > original code would be transformed, and there the 'how it looks' counts, so 
> > in this case it will be treated as a value.
> > Did I misinterpret your question?
> How about 
> https://en.cppreference.com/w/cpp/language/function_template#Explicit_instantiation
>  ?
> I don't see any tests for that.
Also, is there any test coverage missing for 
https://en.cppreference.com/w/cpp/language/template_specialization
and
https://en.cppreference.com/w/cpp/language/partial_specialization
?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54395



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

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

2019-03-17 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

I think this looks good now, one nit about test coverage.
Did you run this through your buildbot, any issues?




Comment at: clang-tidy/utils/FixItHintUtils.cpp:35
+static bool isValueType(QualType QT) { return isValueType(QT.getTypePtr()); }
+static bool isArrayType(QualType QT) { return isa(QT.getTypePtr()); 
}
+static bool isReferenceType(QualType QT) {

JonasToth wrote:
> aaron.ballman wrote:
> > This file is replicating a bunch of functionality that exists on the 
> > `Type*` already. Why do this rather than have the caller do 
> > `QT->isArrayType()` and skip this function entirely? (Same exists elsewhere 
> > here.)
> I had the issue that typedefs are resolved, but shouldn't. If I am not 
> mistaken `QT->isArrayType()` would go to the canconical type.
> 
> ```
> using MyPtr = int*;
> MyPtr foo = nullptr;
> ```
> Is treated wrong in that case.
There is a test for that, right?



Comment at: unittests/clang-tidy/AddConstTest.cpp:733
+  StringRef T = "template  void f(T v) \n";
+  StringRef S = "{ T target = v; }";
+  auto Cat = [](StringRef S) { return (T + S).str(); };

JonasToth wrote:
> alexfh wrote:
> > It would be interesting to see test cases with multiple instantiations of 
> > the template the fix applies to.
> I added test for a template function with many instantiations, but there 
> should not be a difference between the instantiations, because only the 
> original code would be transformed, and there the 'how it looks' counts, so 
> in this case it will be treated as a value.
> Did I misinterpret your question?
How about 
https://en.cppreference.com/w/cpp/language/function_template#Explicit_instantiation
 ?
I don't see any tests for that.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54395



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

[PATCH] D59463: [ASTMatchers][OpenMP] OpenMP Structured-block-related matchers

2019-03-17 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:6421
+///
+/// Prerequisite: the executable directive must not be standalone directive.
+///

lebedev.ri wrote:
> aaron.ballman wrote:
> > What happens if this prereq is not met? Does the matcher return false, or 
> > does it do something worse (like assert)?
> From https://reviews.llvm.org/D59214#change-5GOe7CiMwWxl
> ```
> const Stmt *OMPExecutableDirective::getStructuredBlock() const {
>   assert(!isStandaloneDirective() &&
>  "Standalone Executable Directives don't have Structured Blocks.");
> ```
> It really doesn't make sense to return false/nullptr,
> it would be like trying to find ForLoop in VarDecl, they are just really 
> distinct types.
Hmm, actually, i guess it would be best to return false here instead then. Will 
change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59463



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-17 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

Please always upload all patches with full context (`-U9`)
Misses tests (sema, ast-dump, codegen)




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8164
+def err_likely_attr_invalid_placement : Error<
+  "likely annotation can't apear here">;
+

appear


Repository:
  rC Clang

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

https://reviews.llvm.org/D59467



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