[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

2019-03-24 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

In D35068#1440830 , @koldaniel wrote:

> In D35068#1438498 , @NoQ wrote:
>
> > There seems to be a crash in this code. @koldaniel, would you like to take 
> > a look? https://bugs.llvm.org/show_bug.cgi?id=41185
>
>
> Hi,
>
> True, it is a faulty scenario, my question is what should be the way forward? 
> I think in case of built-in functions there should be no warning, since they 
> differ from the deprecated ones which come from the old standard. The only 
> purpose of the assert was to help development and maintenance (if a new 
> function had been added, it should be decided if it is deprecated or unsafe). 
> Returning instead of asserting would solve the problem.


On many platforms such standard C functions are implemented as macros that 
expand to the respective builtins. I believe there should be no difference in 
behavior between the normal function and the builtin function. Cf. 
`test/Analysis/bstring.c`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D35068



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


[PATCH] D59756: [clangd] Support dependent bases in type hierarchy

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

This patch aims to implement the more proper solution suggested here 
.

I couldn't actually use `CXXRecordDecl->getTemplateInstantiationPattern()` as 
suggested, because the base-specifiers resolve to types, not declarations, but 
I believe what I did here implements the same intention (please let me know if 
I misunderstood).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59756



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


[PATCH] D59756: [clangd] Support dependent bases in type hierarchy

2019-03-24 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision.
nridge added a reviewer: sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric, 
ilya-biryukov.
Herald added a project: clang.

Dependent bases are handled heuristically, by replacing them with the class
template that they are a specialization of, where possible. Care is taken
to avoid infinite recursion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59756

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

Index: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp
===
--- clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp
+++ clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp
@@ -291,9 +291,7 @@
   EXPECT_THAT(typeParents(ChildSpec), ElementsAre(Parent));
 }
 
-// This is disabled for now, because support for dependent bases
-// requires additional measures to avoid infinite recursion.
-TEST(DISABLED_TypeParents, DependentBase) {
+TEST(TypeParents, DependentBase) {
   Annotations Source(R"cpp(
 template 
 struct Parent {};
@@ -386,7 +384,7 @@
 TEST(TypeHierarchy, RecursiveHierarchy1) {
   Annotations Source(R"cpp(
   template 
-  struct S : S {};
+  struct $SDef[[S]] : S {};
 
   S^<0> s;
   )cpp");
@@ -402,14 +400,17 @@
   llvm::Optional Result = getTypeHierarchy(
   AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
-  EXPECT_THAT(*Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct), Parents()));
+  EXPECT_THAT(
+  *Result,
+  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+  SelectionRangeIs(Source.range("SDef")), Parents();
 }
 
 TEST(TypeHierarchy, RecursiveHierarchy2) {
   Annotations Source(R"cpp(
   template 
-  struct S : S {};
+  struct $SDef[[S]] : S {};
 
   template <>
   struct S<0>{};
@@ -426,14 +427,17 @@
   llvm::Optional Result = getTypeHierarchy(
   AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
-  EXPECT_THAT(*Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct), Parents()));
+  EXPECT_THAT(
+  *Result,
+  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+  SelectionRangeIs(Source.range("SDef")), Parents();
 }
 
 TEST(TypeHierarchy, RecursiveHierarchy3) {
   Annotations Source(R"cpp(
   template 
-  struct S : S {};
+  struct $SDef[[S]] : S {};
 
   template <>
   struct S<0>{};
@@ -453,8 +457,11 @@
   llvm::Optional Result = getTypeHierarchy(
   AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
-  EXPECT_THAT(*Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct), Parents()));
+  EXPECT_THAT(
+  *Result,
+  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+  SelectionRangeIs(Source.range("SDef")), Parents();
 }
 
 SymbolID findSymbolIDByName(llvm::StringRef Name, SymbolIndex *Index) {
@@ -599,8 +606,7 @@
   EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(ChildSpec));
 }
 
-// Disabled for the same reason as TypeParents.DependentBase.
-TEST(DISABLED_Subtypes, DependentBase) {
+TEST(Subtypes, DependentBase) {
   Annotations Source(R"cpp(
 template 
 struct Parent {};
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -935,20 +935,40 @@
});
 }
 
-static Optional getTypeAncestors(const CXXRecordDecl &CXXRD,
-ASTContext &ASTCtx) {
+using RecursionProtectionSet = llvm::SmallSet;
+
+static Optional
+getTypeAncestors(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx,
+ RecursionProtectionSet &RPSet) {
   Optional Result = declToTypeHierarchyItem(ASTCtx, CXXRD);
   if (!Result)
 return Result;
 
   Result->parents.emplace();
 
+  // typeParents() will replace dependent template specializations
+  // with their class template, so to avoid infinite recursion for
+  // certain types of hierarchies, keep the templates encountered
+  // along the parent chain in a set, and stop the recursion if one
+  // starts to repeat.
+  auto *Pattern = CXXRD.getDescribedTemplate() ? &CXXRD : nullptr;
+  if (Pattern) {
+if (!RPSet.insert(Pattern).second) {
+  return Result;
+}
+  }
+
   for (const CXXRecordDecl *ParentDecl : typeParents(&CXXRD)) {
 if (Optional ParentSym =
-getTypeAncestors(*ParentDecl, ASTCtx)) {
+getTypeAncestors(*ParentDecl, ASTCtx, RPSet)) {
   Result->parents->emplace_ba

[PATCH] D59743: [WebAssembly] Don't use default GetLinkerPath

2019-03-24 Thread Petr Hosek via Phabricator via cfe-commits
phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/lib/Driver/ToolChains/Fuchsia.h:89
 
-  const char *getDefaultLinker() const override {
-return "ld.lld";
-  }
+  const char *getDefaultLinker() const override { return "ld.lld"; }
 

This seems unrelated?



Comment at: clang/lib/Driver/ToolChains/MipsLinux.h:51
 
-  const char *getDefaultLinker() const override {
-return "ld.lld";
-  }
+  const char *getDefaultLinker() const override { return "ld.lld"; }
 

ditto



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:50
+}
+
+// Since 'wasm-ld' is just another name for lld we accept that name,

I'd also consider handling the `else if (UseLinker.empty() || UseLinker == 
"ld")` case which should return the default linker (i.e. `wasm-ld`), this 
matches the behavior of the generic driver logic in `ToolChain` and can be used 
to select the default linker for the target.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:57
+
+  return ToolChain.GetProgramPath("wasm-ld");
+}

Nit: you could use `getDefaultLinker()` instead of hardcoding `wasm-ld` here so 
if the name ever changes you only need to change the getter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59743



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


[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

2019-03-24 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment.

Thank you for the review!




Comment at: test/SemaCXX/exceptions.cpp:25
+  int ref = k;
+}
 int j = i; // expected-error {{use of undeclared identifier 'i'}}

riccibruno wrote:
> I am wondering if there is already a test which checks systematically that a 
> declaration which shadows another declaration is valid/invalid. I am not 
> seeing such a systematic test, but it might be a nice addition (though that 
> it clearly out of scope for this patch)
That's a good point. I was very surprised when I learned that my original patch 
had broken the lookup behavior described in 
https://bugs.llvm.org/show_bug.cgi?id=41171 but still passed all the tests in 
`check-clang`. I'll try to follow up this patch with some more comprehensive 
tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59752



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


[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

2019-03-24 Thread Brian Gesiak via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC356865: Un-revert "[coroutines][PR40978] Emit error for 
co_yield within catch block" (authored by modocache, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59752?vs=192045&id=192050#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59752

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Scope.h
  lib/Parse/ParseStmt.cpp
  lib/Sema/Scope.cpp
  lib/Sema/SemaCoroutine.cpp
  test/SemaCXX/coroutines.cpp
  test/SemaCXX/exceptions.cpp

Index: test/SemaCXX/exceptions.cpp
===
--- test/SemaCXX/exceptions.cpp
+++ test/SemaCXX/exceptions.cpp
@@ -7,6 +7,7 @@
 struct Abstract { virtual void f() = 0; }; // expected-note {{unimplemented pure virtual method 'f'}}
 
 void trys() {
+  int k = 42;
   try {
   } catch(int i) { // expected-note {{previous definition}}
 int j = i;
@@ -18,6 +19,10 @@
   } catch(A &a) { // expected-error {{cannot catch reference to incomplete type 'A'}}
   } catch(Abstract) { // expected-error {{variable type 'Abstract' is an abstract class}}
   } catch(...) {
+int ref = k;
+{
+  int ref = k;
+}
 int j = i; // expected-error {{use of undeclared identifier 'i'}}
   }
 
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -314,13 +314,23 @@
   }
 };
 
+namespace std { class type_info; }
+
 void unevaluated() {
-  decltype(co_await a); // expected-error {{cannot be used in an unevaluated context}}
-  sizeof(co_await a); // expected-error {{cannot be used in an unevaluated context}}
-  typeid(co_await a); // expected-error {{cannot be used in an unevaluated context}}
-  decltype(co_yield a); // expected-error {{cannot be used in an unevaluated context}}
-  sizeof(co_yield a); // expected-error {{cannot be used in an unevaluated context}}
-  typeid(co_yield a); // expected-error {{cannot be used in an unevaluated context}}
+  decltype(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
+// expected-warning@-1 {{declaration does not declare anything}}
+  sizeof(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
+  // expected-error@-1 {{invalid application of 'sizeof' to an incomplete type 'void'}}
+  typeid(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
+  // expected-warning@-1 {{expression with side effects has no effect in an unevaluated context}}
+  // expected-warning@-2 {{expression result unused}}
+  decltype(co_yield 1); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
+// expected-warning@-1 {{declaration does not declare anything}}
+  sizeof(co_yield 2); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
+  // expected-error@-1 {{invalid application of 'sizeof' to an incomplete type 'void'}}
+  typeid(co_yield 3); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
+  // expected-warning@-1 {{expression with side effects has no effect in an unevaluated context}}
+  // expected-warning@-2 {{expression result unused}}
 }
 
 // [expr.await]p2: "An await-expression shall not appear in a default argument."
@@ -328,6 +338,47 @@
 // not allowed. A user may not understand that this is "outside a function."
 void default_argument(int arg = co_await 0) {} // expected-error {{'co_await' cannot be used outside a function}}
 
+void await_in_catch_coroutine() {
+  try {
+  } catch (...) { // FIXME: Emit a note diagnostic pointing out the try handler on this line.
+[]() -> void { co_await a; }(); // OK
+co_await a; // expected-error {{'co_await' cannot be used in the handler of a try block}}
+  }
+}
+
+void await_nested_in_catch_coroutine() {
+  try {
+  } catch (...) { // FIXME: Emit a note diagnostic pointing out the try handler on this line.
+try {
+  co_await a; // expected-error {{'co_await' cannot be used in the handler of a try block}}
+  []() -> void { co_await a; }(); // OK
+} catch (...) {
+  co_return 123;
+}
+  }
+}
+
+void await_in_lambda_in_catch_coroutine() {
+  try {
+  } catch (...) {
+[]() -> void { co_await a; }(); // OK
+  }
+}
+
+void yield_in_catch_coroutine() {
+  try {
+  } catch (...) {
+co_yield 1; // expected-error {{'co_yield' cannot be used in the handler of a try block}}
+  }
+}
+
+void return_in_catch_coroutine() {
+  try {
+  } catch (...) {
+co_return 123; // OK
+  }
+}
+
 constexpr auto constexpr_deduced_return_coroutine() {
   co_yield 0; // expected-error {{'co_yield' cannot be

r356865 - Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

2019-03-24 Thread Brian Gesiak via cfe-commits
Author: modocache
Date: Sun Mar 24 17:53:10 2019
New Revision: 356865

URL: http://llvm.org/viewvc/llvm-project?rev=356865&view=rev
Log:
Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

Summary:
https://reviews.llvm.org/D59076 added a new coroutine error that
prevented users from using 'co_await' or 'co_yield' within a exception
handler. However, it was reverted in https://reviews.llvm.org/rC356774
because it caused a regression in nested scopes in C++ catch statements,
as documented by https://bugs.llvm.org/show_bug.cgi?id=41171.

The issue was due to an incorrect use of a `clang::ParseScope`. To fix:

1. Add a regression test for catch statement parsing that mimics the bug
   report from https://bugs.llvm.org/show_bug.cgi?id=41171.
2. Re-apply the coroutines error patch from
   https://reviews.llvm.org/D59076, but this time with the correct
   ParseScope behavior.

Reviewers: GorNishanov, tks2103, rsmith, riccibruno, jbulow

Reviewed By: riccibruno

Subscribers: EricWF, jdoerfert, lewissbaker, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Scope.h
cfe/trunk/lib/Parse/ParseStmt.cpp
cfe/trunk/lib/Sema/Scope.cpp
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp
cfe/trunk/test/SemaCXX/exceptions.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=356865&r1=356864&r2=356865&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Mar 24 17:53:10 
2019
@@ -9271,6 +9271,8 @@ def err_coroutine_objc_method : Error<
   "Objective-C methods as coroutines are not yet supported">;
 def err_coroutine_unevaluated_context : Error<
   "'%0' cannot be used in an unevaluated context">;
+def err_coroutine_within_handler : Error<
+  "'%0' cannot be used in the handler of a try block">;
 def err_coroutine_outside_function : Error<
   "'%0' cannot be used outside a function">;
 def err_coroutine_invalid_func_context : Error<

Modified: cfe/trunk/include/clang/Sema/Scope.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Scope.h?rev=356865&r1=356864&r2=356865&view=diff
==
--- cfe/trunk/include/clang/Sema/Scope.h (original)
+++ cfe/trunk/include/clang/Sema/Scope.h Sun Mar 24 17:53:10 2019
@@ -131,6 +131,9 @@ public:
 
 /// We are between inheritance colon and the real class/struct definition 
scope.
 ClassInheritanceScope = 0x80,
+
+/// This is the scope of a C++ catch statement.
+CatchScope = 0x100,
   };
 
 private:

Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=356865&r1=356864&r2=356865&view=diff
==
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Sun Mar 24 17:53:10 2019
@@ -2261,7 +2261,8 @@ StmtResult Parser::ParseCXXCatchBlock(bo
   // The name in a catch exception-declaration is local to the handler and
   // shall not be redeclared in the outermost block of the handler.
   ParseScope CatchScope(this, Scope::DeclScope | Scope::ControlScope |
-  (FnCatch ? Scope::FnTryCatchScope : 0));
+  Scope::CatchScope |
+  (FnCatch ? Scope::FnTryCatchScope : 0));
 
   // exception-declaration is equivalent to '...' or a parameter-declaration
   // without default arguments.

Modified: cfe/trunk/lib/Sema/Scope.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Scope.cpp?rev=356865&r1=356864&r2=356865&view=diff
==
--- cfe/trunk/lib/Sema/Scope.cpp (original)
+++ cfe/trunk/lib/Sema/Scope.cpp Sun Mar 24 17:53:10 2019
@@ -166,7 +166,9 @@ void Scope::dumpImpl(raw_ostream &OS) co
   {SEHExceptScope, "SEHExceptScope"},
   {SEHFilterScope, "SEHFilterScope"},
   {CompoundStmtScope, "CompoundStmtScope"},
-  {ClassInheritanceScope, "ClassInheritanceScope"}};
+  {ClassInheritanceScope, "ClassInheritanceScope"},
+  {CatchScope, "CatchScope"},
+  };
 
   for (auto Info : FlagInfo) {
 if (Flags & Info.first) {

Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=356865&r1=356864&r2=356865&view=diff
==
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Sun Mar 24 17:53:10

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

2019-03-24 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 192049.
hintonda added a comment.

- Fix variable typo/spellings, and add missing flag assignment and additional 
test to catch it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp

Index: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
===
--- clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
+++ clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a %s -verify
+// RUN: %clang_cc1 -std=c++2a %s -verify -pedantic
 
 namespace class_with_ctor {
   struct A { // expected-note 6{{candidate}}
@@ -21,3 +21,21 @@
   C c1 = {{}, {}}; // ok, call default ctor twice
   C c2 = {{1, 2}, {3, 4}}; // expected-error 2{{no matching constructor}}
 }
+
+namespace designator {
+struct A { int x, y; };
+struct B { A a; };
+
+// out of order designators
+A a1 = {.y = 1, .x = 2}; // expected-warning {{designated initializers are a C99 feature}}
+
+// array designator
+int arr[3] = {[1] = 5}; // expected-warning {{designated initializers are a C99 feature}}
+
+// nested designator
+B b = {.a.x = 0}; // expected-warning {{designated initializers are a C99 feature}}
+
+// mixed designator and non-designator
+A a2 = {.x = 1, 2}; // expected-warning {{designated initializers are a C99 feature}}
+A a3 = {1, .y = 2}; // expected-warning {{designated initializers are a C99 feature}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1999,6 +1999,7 @@
   bool CheckForMissingFields =
 !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
   bool HasDesignatedInit = false;
+  bool HasNonDesignatedInit = false;
 
   while (Index < IList->getNumInits()) {
 Expr *Init = IList->getInit(Index);
@@ -2013,6 +2014,10 @@
 
   HasDesignatedInit = true;
 
+  auto LastIdx = Field != FieldEnd
+ ? Field->getFieldIndex()
+ : std::distance(RD->field_begin(), RD->field_end());
+
   // Handle this designated initializer. Field will be updated to
   // the next field that we'll be initializing.
   if (CheckDesignatedInitializer(Entity, IList, DIE, 0,
@@ -2032,6 +2037,15 @@
 }
   }
 
+  // Warn on out of order and mixed designators in C++20.
+  auto NextIdx = Field != FieldEnd
+ ? Field->getFieldIndex()
+ : std::distance(RD->field_begin(), RD->field_end());
+  if (VerifyOnly && (LastIdx >= NextIdx || HasNonDesignatedInit) &&
+  SemaRef.getLangOpts().CPlusPlus2a)
+SemaRef.Diag(Init->getBeginLoc(), diag::ext_designated_init)
+<< Init->getSourceRange();
+
   InitializedSomething = true;
 
   // Disable check for missing fields when designators are used.
@@ -2045,6 +2059,13 @@
   break;
 }
 
+HasNonDesignatedInit = true;
+
+// Warn on mixed designators in C++20
+if (VerifyOnly && HasDesignatedInit && SemaRef.getLangOpts().CPlusPlus2a)
+  SemaRef.Diag(Init->getBeginLoc(), diag::ext_designated_init)
+  << Init->getSourceRange();
+
 // We've already initialized a member of a union. We're done.
 if (InitializedSomething && DeclType->isUnionType())
   break;
@@ -2976,6 +2997,7 @@
   bool Invalid = false;
   SmallVector Designators;
   SmallVector InitExpressions;
+  bool HasArrayDesignator = false;
 
   // Build designators and check array designator expressions.
   for (unsigned Idx = 0; Idx < Desig.getNumDesignators(); ++Idx) {
@@ -2999,6 +3021,7 @@
 D.getRBracketLoc()));
 InitExpressions.push_back(Index);
   }
+  HasArrayDesignator = true;
   break;
 }
 
@@ -3042,6 +3065,7 @@
   InitExpressions.push_back(EndIndex);
 }
   }
+  HasArrayDesignator = true;
   break;
 }
 }
@@ -3060,8 +3084,11 @@
  Init.getAs());
 
   if (!getLangOpts().C99)
-Diag(DIE->getBeginLoc(), diag::ext_designated_init)
-<< DIE->getSourceRange();
+// Warn on nested and array designators in C++20
+if (!getLangOpts().CPlusPlus2a || Desig.getNumDesignators() > 1 ||
+HasArrayDesignator)
+  Diag(DIE->getBeginLoc(), diag::ext_designated_init)
+  << DIE->getSourceRange();
 
   return DIE;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D59745: [NFC] Move writeFuncOrVarName out of class CodegenNameGenerator so that it can be reused more easily.

2019-03-24 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi abandoned this revision.
plotfi added a comment.

Found a better way to do what I needed without this unnecessary change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59745



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


[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

2019-03-24 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:2002
   bool HasDesignatedInit = false;
+  bool HasNonDesginatedInit = false;
 

typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754



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


[PATCH] D59754: [Sema] Add c++2a designator initializer warnings

2019-03-24 Thread Don Hinton via Phabricator via cfe-commits
hintonda created this revision.
hintonda added reviewers: rsmith, xbolva00.
Herald added a project: clang.

Basic designator initializers are allowed in c++2a, so only
issue warnings for the extended ones that aren't allowed:

  - out of order
- nested
- arrays
- mixed


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59754

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp

Index: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
===
--- clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
+++ clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a %s -verify
+// RUN: %clang_cc1 -std=c++2a %s -verify -pedantic
 
 namespace class_with_ctor {
   struct A { // expected-note 6{{candidate}}
@@ -21,3 +21,20 @@
   C c1 = {{}, {}}; // ok, call default ctor twice
   C c2 = {{1, 2}, {3, 4}}; // expected-error 2{{no matching constructor}}
 }
+
+namespace designator {
+struct A { int x, y; };
+struct B { A a; };
+
+// out of order designators
+A a1 = {.y = 1, .x = 2}; // expected-warning {{designated initializers are a C99 feature}}
+
+// array designator
+int arr[3] = {[1] = 5}; // expected-warning {{designated initializers are a C99 feature}}
+
+// nested designator
+B b = {.a.x = 0}; // expected-warning {{designated initializers are a C99 feature}}
+
+// mixed designator and non-designator
+A a2 = {.x = 1, 2}; // expected-warning {{designated initializers are a C99 feature}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1999,6 +1999,7 @@
   bool CheckForMissingFields =
 !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
   bool HasDesignatedInit = false;
+  bool HasNonDesginatedInit = false;
 
   while (Index < IList->getNumInits()) {
 Expr *Init = IList->getInit(Index);
@@ -2013,6 +2014,10 @@
 
   HasDesignatedInit = true;
 
+  auto LastIdx = Field != FieldEnd
+ ? Field->getFieldIndex()
+ : std::distance(RD->field_begin(), RD->field_end());
+
   // Handle this designated initializer. Field will be updated to
   // the next field that we'll be initializing.
   if (CheckDesignatedInitializer(Entity, IList, DIE, 0,
@@ -2032,6 +2037,15 @@
 }
   }
 
+  // Warn on out of order and mixed designators in C++20.
+  auto NextIdx = Field != FieldEnd
+ ? Field->getFieldIndex()
+ : std::distance(RD->field_begin(), RD->field_end());
+  if (VerifyOnly && (LastIdx >= NextIdx || HasNonDesginatedInit) &&
+  SemaRef.getLangOpts().CPlusPlus2a)
+SemaRef.Diag(Init->getBeginLoc(), diag::ext_designated_init)
+<< Init->getSourceRange();
+
   InitializedSomething = true;
 
   // Disable check for missing fields when designators are used.
@@ -2045,6 +2059,11 @@
   break;
 }
 
+// Warn on mixed designators in C++20
+if (VerifyOnly && HasDesignatedInit && SemaRef.getLangOpts().CPlusPlus2a)
+  SemaRef.Diag(Init->getBeginLoc(), diag::ext_designated_init)
+  << Init->getSourceRange();
+
 // We've already initialized a member of a union. We're done.
 if (InitializedSomething && DeclType->isUnionType())
   break;
@@ -2976,6 +2995,7 @@
   bool Invalid = false;
   SmallVector Designators;
   SmallVector InitExpressions;
+  bool HaveArrayDesignator = false;
 
   // Build designators and check array designator expressions.
   for (unsigned Idx = 0; Idx < Desig.getNumDesignators(); ++Idx) {
@@ -2999,6 +3019,7 @@
 D.getRBracketLoc()));
 InitExpressions.push_back(Index);
   }
+  HaveArrayDesignator = true;
   break;
 }
 
@@ -3042,6 +3063,7 @@
   InitExpressions.push_back(EndIndex);
 }
   }
+  HaveArrayDesignator = true;
   break;
 }
 }
@@ -3060,8 +3082,11 @@
  Init.getAs());
 
   if (!getLangOpts().C99)
-Diag(DIE->getBeginLoc(), diag::ext_designated_init)
-<< DIE->getSourceRange();
+// Warn on nested and array designators in C++20
+if (!getLangOpts().CPlusPlus2a || Desig.getNumDesignators() > 1 ||
+HaveArrayDesignator)
+  Diag(DIE->getBeginLoc(), diag::ext_designated_init)
+  << DIE->getSourceRange();
 
   return DIE;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

2019-03-24 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno accepted this revision.
riccibruno added a comment.
This revision is now accepted and ready to land.

Great! Since this already received one round of reviews I guess this looks okay.




Comment at: test/SemaCXX/exceptions.cpp:25
+  int ref = k;
+}
 int j = i; // expected-error {{use of undeclared identifier 'i'}}

I am wondering if there is already a test which checks systematically that a 
declaration which shadows another declaration is valid/invalid. I am not seeing 
such a systematic test, but it might be a nice addition (though that it clearly 
out of scope for this patch)


Repository:
  rC Clang

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

https://reviews.llvm.org/D59752



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


RE: [clang-tools-extra] r356849 - [pp-trace] Modernize the code

2019-03-24 Thread via cfe-commits
Hi Fangrui,

This change is causing the PS4 Windows bot to fail to build:

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/24560/steps/build/logs/stdio

FAILED: 
tools/clang/tools/extra/pp-trace/CMakeFiles/pp-trace.dir/PPTrace.cpp.obj 
C:\PROGRA~2\MIB055~1\2017\COMMUN~1\VC\Tools\MSVC\1416~1.270\bin\Hostx64\x64\cl.exe
  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE 
-D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS 
-D_GNU_SOURCE -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE 
-D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS 
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\clang\tools\extra\pp-trace 
-IC:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\tools\extra\pp-trace
 
-IC:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\include
 -Itools\clang\include -Iinclude 
-IC:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\include
 /DWIN32 /D_WINDOWS   /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /W4 
-wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 
-wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 
-wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 
-wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd4324 
-w14062 -we4238 /MD /O2 /Ob2   -UNDEBUG  /EHs-c- /GR- /showIncludes 
/Fotools\clang\tools\extra\pp-trace\CMakeFiles\pp-trace.dir\PPTrace.cpp.obj 
/Fdtools\clang\tools\extra\pp-trace\CMakeFiles\pp-trace.dir\ /FS -c 
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\tools\extra\pp-trace\PPTrace.cpp
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\tools\extra\pp-trace\PPTrace.cpp(93):
 error C2668: 'llvm::make_unique': ambiguous call to overloaded function
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\include\llvm/ADT/STLExtras.h(1348):
 note: could be 'std::unique_ptr> 
llvm::make_unique>&,clang::Preprocessor&>(const
 FilterType &,std::vector> 
&,clang::Preprocessor &)'
with
[
_Ty=PPCallbacksTracker
]
C:\Program Files (x86)\Microsoft Visual 
Studio\2017\Community\VC\Tools\MSVC\14.16.27023\include\memory(2537): note: or  
 'std::unique_ptr> 
std::make_unique>&,clang::Preprocessor&,0>(const
 FilterType &,std::vector> 
&,clang::Preprocessor &)' [found using argument-dependent lookup]
with
[
_Ty=PPCallbacksTracker
]
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\tools\extra\pp-trace\PPTrace.cpp(93):
 note: while trying to match the argument list '(const FilterType, 
std::vector>, clang::Preprocessor)'
with
[
_Ty=CallbackCall
]
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\tools\extra\pp-trace\PPTrace.cpp(155):
 error C2668: 'llvm::make_unique': ambiguous call to overloaded function
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\include\llvm/ADT/STLExtras.h(1348):
 note: could be 
'std::unique_ptr<`anonymous-namespace'::PPTraceFrontendActionFactory,std::default_delete<_Ty>>
 
llvm::make_unique<`anonymous-namespace'::PPTraceFrontendActionFactory,FilterType&,llvm::raw_fd_ostream&>(FilterType
 &,llvm::raw_fd_ostream &)'
with
[
_Ty=`anonymous-namespace'::PPTraceFrontendActionFactory
]
C:\Program Files (x86)\Microsoft Visual 
Studio\2017\Community\VC\Tools\MSVC\14.16.27023\include\memory(2537): note: or  
 
'std::unique_ptr<`anonymous-namespace'::PPTraceFrontendActionFactory,std::default_delete<_Ty>>
 
std::make_unique<`anonymous-namespace'::PPTraceFrontendActionFactory,FilterType&,llvm::raw_fd_ostream&,0>(FilterType
 &,llvm::raw_fd_ostream &)' [found using argument-dependent lookup]
with
[
_Ty=`anonymous-namespace'::PPTraceFrontendActionFactory
]
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\tools\extra\pp-trace\PPTrace.cpp(155):
 note: while trying to match the argument list '(FilterType, 
llvm::raw_fd_ostream)'
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\tools\extra\pp-trace\PPTrace.cpp(155):
 error C2248: 'llvm::Error::Error': cannot access protected member declared in 
class 'llvm::Error'
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\include\llvm/Support/Error.h(176):
 note: see declaration of 'llvm::Error::Error'
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\include\llvm/Support/Error.h(157):
 note: see declaration of 'llvm::Error'

Can you take a look?

Douglas Yung

-Original Message-
From: cfe-commits  On Behalf Of Fangrui 

[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

2019-03-24 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added inline comments.



Comment at: lib/Parse/ParseStmt.cpp:2293
   // FIXME: Possible draft standard bug: attribute-specifier should be allowed?
   StmtResult Block(ParseCompoundStatement());
   if (Block.isInvalid())

riccibruno wrote:
> Just to make sure I understood the problem correctly, the issue was that you 
> passed `ScopeFlags` to `ParseCompoundStatement()`. This override the default 
> flags which are `Scope::DeclScope | Scope::CompoundStmtScope`. In particular 
> now `ParseCompoundStatement()` was done as if in the controlling scope of an 
> if statement. Now as per [basic.scope.block]/p4:
> 
> > Names declared in the init-statement, the for-range-declaration, and in the 
> > condition of if, while, for, and switch statements are local to the if, 
> > while, for, or switch statement (including the controlled statement), and 
> > shall not be redeclared in a subsequent condition of that statement nor in 
> > the outermost block (or, for the if statement, any of the outermost blocks) 
> > of the controlled statement; see 9.4.
> 
> This caused the declaration in the compound statement to be detected as 
> erroneous. Indeed the following worked just fine. 
> 
> ```
> void f() {
> try {}
> catch (...) {
> int i;
> {
> {
> int i;
> }
> }
> }
> }
> ```
> 
> Does this make sense or I am completely off base here ?
> 
That's exactly my understanding, yes! In fact thank you for the clear 
explanation.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59752



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


[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

2019-03-24 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added inline comments.



Comment at: lib/Parse/ParseStmt.cpp:2293
   // FIXME: Possible draft standard bug: attribute-specifier should be allowed?
   StmtResult Block(ParseCompoundStatement());
   if (Block.isInvalid())

Just to make sure I understood the problem correctly, the issue was that you 
passed `ScopeFlags` to `ParseCompoundStatement()`. This override the default 
flags which are `Scope::DeclScope | Scope::CompoundStmtScope`. In particular 
now `ParseCompoundStatement()` was done as if in the controlling scope of an if 
statement. Now as per [basic.scope.block]/p4:

> Names declared in the init-statement, the for-range-declaration, and in the 
> condition of if, while, for, and switch statements are local to the if, 
> while, for, or switch statement (including the controlled statement), and 
> shall not be redeclared in a subsequent condition of that statement nor in 
> the outermost block (or, for the if statement, any of the outermost blocks) 
> of the controlled statement; see 9.4.

This caused the declaration in the compound statement to be detected as 
erroneous. Indeed the following worked just fine. 

```
void f() {
try {}
catch (...) {
int i;
{
{
int i;
}
}
}
}
```

Does this make sense or I am completely off base here ?



Repository:
  rC Clang

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

https://reviews.llvm.org/D59752



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


[PATCH] D16761: clang-cl: Support loading plugins on Windows

2019-03-24 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

LLVM_EXPORT_REGISTRY no longer exists, so this can't be relanded.


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

https://reviews.llvm.org/D16761



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


[PATCH] D59750: Rename directory housing clang-include-fixer to be eponymous

2019-03-24 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision.
bkramer added a comment.
This revision is now accepted and ready to land.

lg. Is the reference from libclang still around? Might need an update.


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

https://reviews.llvm.org/D59750



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


Re: r356862 - [X86] Make _bswap intrinsic a function instead of a macro to hopefully fix the chromium build.

2019-03-24 Thread Nico Weber via cfe-commits
Thanks, this helped.

> to hopefully fix the chromium build.

I'd argue it's overall more sane. Single underscore followed by lower-case
letters isn't reserved in non-global scope, so what gcc is doing isn't
quite proper from what I understand.

On Sun, Mar 24, 2019 at 1:58 PM Craig Topper via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ctopper
> Date: Sun Mar 24 11:00:20 2019
> New Revision: 356862
>
> URL: http://llvm.org/viewvc/llvm-project?rev=356862&view=rev
> Log:
> [X86] Make _bswap intrinsic a function instead of a macro to hopefully fix
> the chromium build.
>
> This intrinsic was added in r356848 but was implemented as a macro to
> match gcc.
>
> Modified:
> cfe/trunk/lib/Headers/ia32intrin.h
>
> Modified: cfe/trunk/lib/Headers/ia32intrin.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/ia32intrin.h?rev=356862&r1=356861&r2=356862&view=diff
>
> ==
> --- cfe/trunk/lib/Headers/ia32intrin.h (original)
> +++ cfe/trunk/lib/Headers/ia32intrin.h Sun Mar 24 11:00:20 2019
> @@ -78,7 +78,11 @@ __bswapd(int __A) {
>return __builtin_bswap32(__A);
>  }
>
> -#define _bswap(A) __bswapd((A))
> +static __inline__ int __attribute__((__always_inline__, __nodebug__))
> +_bswap(int __A) {
> +  return __builtin_bswap32(__A);
> +}
> +
>  #define _bit_scan_forward(A) __bsfd((A))
>  #define _bit_scan_reverse(A) __bsrd((A))
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D59745: [NFC] Move writeFuncOrVarName out of class CodegenNameGenerator so that it can be reused more easily.

2019-03-24 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added a comment.

Hmm, what exactly does the libclang interfaces not give you or what exactly did 
you intend to have this be used as.  Perhaps with some more details we can find 
a good solution for the specific case that you have in mind.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59745



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


[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"

2019-03-24 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision.
modocache added reviewers: GorNishanov, tks2103, rsmith.
Herald added subscribers: jdoerfert, EricWF.
Herald added a project: clang.

https://reviews.llvm.org/D59076 added a new coroutine error that
prevented users from using 'co_await' or 'co_yield' within a exception
handler. However, it was reverted in https://reviews.llvm.org/rC356774
because it caused a regression in nested scopes in C++ catch statements,
as documented by https://bugs.llvm.org/show_bug.cgi?id=41171.

The issue was due to an incorrect use of a `clang::ParseScope`. To fix:

1. Add a regression test for catch statement parsing that mimics the bug report 
from https://bugs.llvm.org/show_bug.cgi?id=41171.
2. Re-apply the coroutines error patch from https://reviews.llvm.org/D59076, 
but this time with the correct ParseScope behavior.


Repository:
  rC Clang

https://reviews.llvm.org/D59752

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Scope.h
  lib/Parse/ParseStmt.cpp
  lib/Sema/Scope.cpp
  lib/Sema/SemaCoroutine.cpp
  test/SemaCXX/coroutines.cpp
  test/SemaCXX/exceptions.cpp

Index: test/SemaCXX/exceptions.cpp
===
--- test/SemaCXX/exceptions.cpp
+++ test/SemaCXX/exceptions.cpp
@@ -7,6 +7,7 @@
 struct Abstract { virtual void f() = 0; }; // expected-note {{unimplemented pure virtual method 'f'}}
 
 void trys() {
+  int k = 42;
   try {
   } catch(int i) { // expected-note {{previous definition}}
 int j = i;
@@ -18,6 +19,10 @@
   } catch(A &a) { // expected-error {{cannot catch reference to incomplete type 'A'}}
   } catch(Abstract) { // expected-error {{variable type 'Abstract' is an abstract class}}
   } catch(...) {
+int ref = k;
+{
+  int ref = k;
+}
 int j = i; // expected-error {{use of undeclared identifier 'i'}}
   }
 
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -314,13 +314,23 @@
   }
 };
 
+namespace std { class type_info; }
+
 void unevaluated() {
-  decltype(co_await a); // expected-error {{cannot be used in an unevaluated context}}
-  sizeof(co_await a); // expected-error {{cannot be used in an unevaluated context}}
-  typeid(co_await a); // expected-error {{cannot be used in an unevaluated context}}
-  decltype(co_yield a); // expected-error {{cannot be used in an unevaluated context}}
-  sizeof(co_yield a); // expected-error {{cannot be used in an unevaluated context}}
-  typeid(co_yield a); // expected-error {{cannot be used in an unevaluated context}}
+  decltype(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
+// expected-warning@-1 {{declaration does not declare anything}}
+  sizeof(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
+  // expected-error@-1 {{invalid application of 'sizeof' to an incomplete type 'void'}}
+  typeid(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
+  // expected-warning@-1 {{expression with side effects has no effect in an unevaluated context}}
+  // expected-warning@-2 {{expression result unused}}
+  decltype(co_yield 1); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
+// expected-warning@-1 {{declaration does not declare anything}}
+  sizeof(co_yield 2); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
+  // expected-error@-1 {{invalid application of 'sizeof' to an incomplete type 'void'}}
+  typeid(co_yield 3); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
+  // expected-warning@-1 {{expression with side effects has no effect in an unevaluated context}}
+  // expected-warning@-2 {{expression result unused}}
 }
 
 // [expr.await]p2: "An await-expression shall not appear in a default argument."
@@ -328,6 +338,47 @@
 // not allowed. A user may not understand that this is "outside a function."
 void default_argument(int arg = co_await 0) {} // expected-error {{'co_await' cannot be used outside a function}}
 
+void await_in_catch_coroutine() {
+  try {
+  } catch (...) { // FIXME: Emit a note diagnostic pointing out the try handler on this line.
+[]() -> void { co_await a; }(); // OK
+co_await a; // expected-error {{'co_await' cannot be used in the handler of a try block}}
+  }
+}
+
+void await_nested_in_catch_coroutine() {
+  try {
+  } catch (...) { // FIXME: Emit a note diagnostic pointing out the try handler on this line.
+try {
+  co_await a; // expected-error {{'co_await' cannot be used in the handler of a try block}}
+  []() -> void { co_await a; }(); // OK
+} catch (...) {
+  co_return 123;
+}
+  }
+}
+
+void await_in_lambda_in_cat

[PATCH] D16761: clang-cl: Support loading plugins on Windows

2019-03-24 Thread NAKAMURA Takumi via Phabricator via cfe-commits
chapuni added a comment.

@v.g.vassilev, I couldn't explain what it caused. Fell free to recommit.


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

https://reviews.llvm.org/D16761



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


[PATCH] D16761: clang-cl: Support loading plugins on Windows

2019-03-24 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added subscribers: chapuni, v.g.vassilev.
v.g.vassilev added a comment.

@chapuni, you have reverted this patch and the commit message says 
"investigating". Did you come to a conclusion? Can we reland it?


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

https://reviews.llvm.org/D16761



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


[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

2019-03-24 Thread Alexey Bader via Phabricator via cfe-commits
bader added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:11648-11651
+// In OpenCL we don't allow to initialize objects in local address space.
+if (getLangOpts().OpenCL &&
+Var->getType().getAddressSpace() == LangAS::opencl_local)
+  return;

Shouldn't we invalidate Var declaration?



Comment at: test/CodeGenOpenCLCXX/addrspace-of-this.cl:154
 
-// Test the address space of 'this' when invoking a constructor for an object 
in non-default address space
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// Test that we don't initialize an onbject in local address space
+// EXPL-NOT: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))

onbject  -> object



Comment at: test/CodeGenOpenCLCXX/local_addrspace_init.cl:12
+  __local int i;
+  __local C ii;
+  //FIXME: In OpenCL C we don't accept initializers for local address space 
variables.

I guess this declaration should be disallowed for non-POD types. Can we add a 
check for that to some test/Sema* test?


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

https://reviews.llvm.org/D59646



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


[PATCH] D55049: Changed every use of ASTImporter::Import to Import_New

2019-03-24 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment.

Hi Balazs,

The looks mostly good to me.




Comment at: lib/AST/ASTImporter.cpp:3440
 
-  for (const auto *Attr : D->attrs())
-ToIndirectField->addAttr(Importer.Import(Attr));

There is the same deletion in D53757.



Comment at: lib/AST/ASTImporter.cpp:8550
+if (ExpectedType ToFromOrErr = Import_New(From)) {
+  if (ToContext.hasSameType(*ToFromOrErr, To))
+return true;

Wow, we import types instead of just checking them for structural equivalence. 
That's OK to leave it in the patch as-is but looks pretty strange. Maybe this 
even deserves a FIXME.



Comment at: unittests/AST/ASTImporterTest.cpp:146
+ << "Import failed, error: \"" << Err << "\"!";
+  llvm::consumeError(std::move(Err));
+}

Dead code?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55049



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


[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

2019-03-24 Thread Ronald Wampler via Phabricator via cfe-commits
rdwampler added inline comments.



Comment at: clang/include/clang/Format/Format.h:264
+/// If Else statements have no braces don't put them
+/// on the same line.
+/// \code

These comments can be used to auto generate the corresponding sections in 
clang/docs/ClangFormatStyleOptions.rst using 
clang/docs/tools/dump_format_style.py, so they should probably be kept in sync.


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

https://reviews.llvm.org/D59408



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


[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

2019-03-24 Thread Dave Green via Phabricator via cfe-commits
dmgreen added a comment.

Hello. I also don't feel very familiar with clang, but had a poke around and I 
think it looks pretty good. I see unroll and jam is being awkward again.

This could maybe do with a few extra tests. Am I correct in saying something 
like this:

  #pragma unroll_and_jam(4)
  for(int j = 0; j < n; j++) {
#pragma unroll(4)
for(int k = 0; k < n; k++) {
  x[j*n+k] = 10;
}
  }

would end up with a llvm.loop.unroll_and_jam.followup_inner with a 
llvm.loop.unroll_count?




Comment at: lib/CodeGen/CGLoopInfo.cpp:500
+// Unroll-and-jam of an inner loop and unroll-and-jam of the same loop as
+// the outer loop does not make much sense, but we have to pick an order.
+AfterJam.UnrollAndJamCount = Attrs.UnrollAndJamCount;

I was having trouble parsing this sentance. Does it mean both the inner loop 
and the outer loop both have unroll-and-jam? UnrollAndJam processes loops from 
inner to outer, so if this is working as I think, maybe it should be put into 
BeforeJam.



Comment at: lib/CodeGen/CGLoopInfo.cpp:502
+AfterJam.UnrollAndJamCount = Attrs.UnrollAndJamCount;
+AfterJam.UnrollAndJamEnable = AfterJam.UnrollAndJamEnable;
+

 = Attrs.UnrollAndJamEnable ?



Comment at: lib/CodeGen/CGLoopInfo.h:135
+  ///  LoopID.
+  /// @param HasUserTransforms [out] Set to true if the returned MDNode 
encoodes
+  ///  at least one transformation.

*encodes



Comment at: lib/CodeGen/CGLoopInfo.h:170
+  /// @param Attrs This loop's attributes and transfomations.
+  /// @param HasUserTransforms [out] Set to true if the returned MDNode 
encoodes
+  ///  at least one transformation.

*encodes


Repository:
  rC Clang

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

https://reviews.llvm.org/D57978



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


Re: r356848 - [X86] Add BSR/BSF/BSWAP intrinsics to ia32intrin.h to match gcc.

2019-03-24 Thread Craig Topper via cfe-commits
Made a function in r356852

~Craig


On Sun, Mar 24, 2019 at 9:59 AM Nico Weber via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> This breaks Chromium's build. We have a class with a _bswap method, and
> this adds a _bswap macro expanding to something that gets in the way. Could
> _bswap be an inline function instead?
>
> https://bugs.chromium.org/p/chromium/issues/detail?id=945172
>
> On Sat, Mar 23, 2019 at 8:55 PM Craig Topper via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: ctopper
>> Date: Sat Mar 23 17:56:52 2019
>> New Revision: 356848
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=356848&view=rev
>> Log:
>> [X86] Add BSR/BSF/BSWAP intrinsics to ia32intrin.h to match gcc.
>>
>> Summary:
>> These are all implemented by icc as well.
>>
>> I made bit_scan_forward/reverse forward to the __bsfd/__bsrq since we
>> also have
>> __bsfq/__bsrq.
>>
>> Note, when lzcnt is enabled the bsr intrinsics generates lzcnt+xor
>> instead of bsr.
>>
>> Reviewers: RKSimon, spatel
>>
>> Subscribers: cfe-commits, llvm-commits
>>
>> Tags: #clang
>>
>> Differential Revision: https://reviews.llvm.org/D59682
>>
>> Added:
>> cfe/trunk/test/CodeGen/x86-bswap.c
>> Modified:
>> cfe/trunk/lib/Headers/ia32intrin.h
>> cfe/trunk/lib/Headers/immintrin.h
>> cfe/trunk/test/CodeGen/bitscan-builtins.c
>>
>> Modified: cfe/trunk/lib/Headers/ia32intrin.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/ia32intrin.h?rev=356848&r1=356847&r2=356848&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Headers/ia32intrin.h (original)
>> +++ cfe/trunk/lib/Headers/ia32intrin.h Sat Mar 23 17:56:52 2019
>> @@ -28,6 +28,114 @@
>>  #ifndef __IA32INTRIN_H
>>  #define __IA32INTRIN_H
>>
>> +/** Find the first set bit starting from the lsb. Result is undefined if
>> + *  input is 0.
>> + *
>> + *  \headerfile 
>> + *
>> + *  This intrinsic corresponds to the  BSF  instruction or the
>> + *   TZCNT  instruction.
>> + *
>> + *  \param __A
>> + * A 32-bit integer operand.
>> + *  \returns A 32-bit integer containing the bit number.
>> + */
>> +static __inline__ int __attribute__((__always_inline__, __nodebug__))
>> +__bsfd(int __A) {
>> +  return __builtin_ctz(__A);
>> +}
>> +
>> +/** Find the first set bit starting from the msb. Result is undefined if
>> + *  input is 0.
>> + *
>> + *  \headerfile 
>> + *
>> + *  This intrinsic corresponds to the  BSR  instruction or the
>> + *   LZCNT  instruction and an  XOR .
>> + *
>> + *  \param __A
>> + * A 32-bit integer operand.
>> + *  \returns A 32-bit integer containing the bit number.
>> + */
>> +static __inline__ int __attribute__((__always_inline__, __nodebug__))
>> +__bsrd(int __A) {
>> +  return 31 - __builtin_clz(__A);
>> +}
>> +
>> +/** Swaps the bytes in the input. Converting little endian to big endian
>> or
>> + *  vice versa.
>> + *
>> + *  \headerfile 
>> + *
>> + *  This intrinsic corresponds to the  BSWAP  instruction.
>> + *
>> + *  \param __A
>> + * A 32-bit integer operand.
>> + *  \returns A 32-bit integer containing the swapped bytes.
>> + */
>> +static __inline__ int __attribute__((__always_inline__, __nodebug__))
>> +__bswapd(int __A) {
>> +  return __builtin_bswap32(__A);
>> +}
>> +
>> +#define _bswap(A) __bswapd((A))
>> +#define _bit_scan_forward(A) __bsfd((A))
>> +#define _bit_scan_reverse(A) __bsrd((A))
>> +
>> +#ifdef __x86_64__
>> +/** Find the first set bit starting from the lsb. Result is undefined if
>> + *  input is 0.
>> + *
>> + *  \headerfile 
>> + *
>> + *  This intrinsic corresponds to the  BSF  instruction or the
>> + *   TZCNT  instruction.
>> + *
>> + *  \param __A
>> + * A 64-bit integer operand.
>> + *  \returns A 32-bit integer containing the bit number.
>> + */
>> +static __inline__ int __attribute__((__always_inline__, __nodebug__))
>> +__bsfq(long long __A) {
>> +  return __builtin_ctzll(__A);
>> +}
>> +
>> +/** Find the first set bit starting from the msb. Result is undefined if
>> + *  input is 0.
>> + *
>> + *  \headerfile 
>> + *
>> + *  This intrinsic corresponds to the  BSR  instruction or the
>> + *   LZCNT  instruction and an  XOR .
>> + *
>> + *  \param __A
>> + * A 64-bit integer operand.
>> + *  \returns A 32-bit integer containing the bit number.
>> + */
>> +static __inline__ int __attribute__((__always_inline__, __nodebug__))
>> +__bsrq(long long __A) {
>> +  return 63 - __builtin_clzll(__A);
>> +}
>> +
>> +/** Swaps the bytes in the input. Converting little endian to big endian
>> or
>> + *  vice versa.
>> + *
>> + *  \headerfile 
>> + *
>> + *  This intrinsic corresponds to the  BSWAP  instruction.
>> + *
>> + *  \param __A
>> + * A 64-bit integer operand.
>> + *  \returns A 64-bit integer containing the swapped bytes.
>> + */
>> +static __inline__ long long __attribute__((__always_inline__,
>> __nodebug__))
>> +__bswapq(long long __A) {
>> +  return __builtin_bswap64(__A);
>> +}
>> +
>> +#define _bswap64(A

r356862 - [X86] Make _bswap intrinsic a function instead of a macro to hopefully fix the chromium build.

2019-03-24 Thread Craig Topper via cfe-commits
Author: ctopper
Date: Sun Mar 24 11:00:20 2019
New Revision: 356862

URL: http://llvm.org/viewvc/llvm-project?rev=356862&view=rev
Log:
[X86] Make _bswap intrinsic a function instead of a macro to hopefully fix the 
chromium build.

This intrinsic was added in r356848 but was implemented as a macro to match gcc.

Modified:
cfe/trunk/lib/Headers/ia32intrin.h

Modified: cfe/trunk/lib/Headers/ia32intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/ia32intrin.h?rev=356862&r1=356861&r2=356862&view=diff
==
--- cfe/trunk/lib/Headers/ia32intrin.h (original)
+++ cfe/trunk/lib/Headers/ia32intrin.h Sun Mar 24 11:00:20 2019
@@ -78,7 +78,11 @@ __bswapd(int __A) {
   return __builtin_bswap32(__A);
 }
 
-#define _bswap(A) __bswapd((A))
+static __inline__ int __attribute__((__always_inline__, __nodebug__))
+_bswap(int __A) {
+  return __builtin_bswap32(__A);
+}
+
 #define _bit_scan_forward(A) __bsfd((A))
 #define _bit_scan_reverse(A) __bsrd((A))
 


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


[PATCH] D59750: Rename directory housing clang-include-fixer to be eponymous

2019-03-24 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

I accidentally hit some keyboard shortcut and created the revision before I was 
done adding reviewer and subscriber lists. Fixed now; apologies for the 
confusing email this will send.


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

https://reviews.llvm.org/D59750



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


[PATCH] D59298: [RISCV] Pass -target-abi to -cc1as

2019-03-24 Thread Alex Bradbury via Phabricator via cfe-commits
asb accepted this revision.
asb added a comment.
This revision is now accepted and ready to land.
Herald added subscribers: benna, psnobl.

Thanks, LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59298



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


Re: r356848 - [X86] Add BSR/BSF/BSWAP intrinsics to ia32intrin.h to match gcc.

2019-03-24 Thread Nico Weber via cfe-commits
This breaks Chromium's build. We have a class with a _bswap method, and
this adds a _bswap macro expanding to something that gets in the way. Could
_bswap be an inline function instead?

https://bugs.chromium.org/p/chromium/issues/detail?id=945172

On Sat, Mar 23, 2019 at 8:55 PM Craig Topper via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ctopper
> Date: Sat Mar 23 17:56:52 2019
> New Revision: 356848
>
> URL: http://llvm.org/viewvc/llvm-project?rev=356848&view=rev
> Log:
> [X86] Add BSR/BSF/BSWAP intrinsics to ia32intrin.h to match gcc.
>
> Summary:
> These are all implemented by icc as well.
>
> I made bit_scan_forward/reverse forward to the __bsfd/__bsrq since we also
> have
> __bsfq/__bsrq.
>
> Note, when lzcnt is enabled the bsr intrinsics generates lzcnt+xor instead
> of bsr.
>
> Reviewers: RKSimon, spatel
>
> Subscribers: cfe-commits, llvm-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D59682
>
> Added:
> cfe/trunk/test/CodeGen/x86-bswap.c
> Modified:
> cfe/trunk/lib/Headers/ia32intrin.h
> cfe/trunk/lib/Headers/immintrin.h
> cfe/trunk/test/CodeGen/bitscan-builtins.c
>
> Modified: cfe/trunk/lib/Headers/ia32intrin.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/ia32intrin.h?rev=356848&r1=356847&r2=356848&view=diff
>
> ==
> --- cfe/trunk/lib/Headers/ia32intrin.h (original)
> +++ cfe/trunk/lib/Headers/ia32intrin.h Sat Mar 23 17:56:52 2019
> @@ -28,6 +28,114 @@
>  #ifndef __IA32INTRIN_H
>  #define __IA32INTRIN_H
>
> +/** Find the first set bit starting from the lsb. Result is undefined if
> + *  input is 0.
> + *
> + *  \headerfile 
> + *
> + *  This intrinsic corresponds to the  BSF  instruction or the
> + *   TZCNT  instruction.
> + *
> + *  \param __A
> + * A 32-bit integer operand.
> + *  \returns A 32-bit integer containing the bit number.
> + */
> +static __inline__ int __attribute__((__always_inline__, __nodebug__))
> +__bsfd(int __A) {
> +  return __builtin_ctz(__A);
> +}
> +
> +/** Find the first set bit starting from the msb. Result is undefined if
> + *  input is 0.
> + *
> + *  \headerfile 
> + *
> + *  This intrinsic corresponds to the  BSR  instruction or the
> + *   LZCNT  instruction and an  XOR .
> + *
> + *  \param __A
> + * A 32-bit integer operand.
> + *  \returns A 32-bit integer containing the bit number.
> + */
> +static __inline__ int __attribute__((__always_inline__, __nodebug__))
> +__bsrd(int __A) {
> +  return 31 - __builtin_clz(__A);
> +}
> +
> +/** Swaps the bytes in the input. Converting little endian to big endian
> or
> + *  vice versa.
> + *
> + *  \headerfile 
> + *
> + *  This intrinsic corresponds to the  BSWAP  instruction.
> + *
> + *  \param __A
> + * A 32-bit integer operand.
> + *  \returns A 32-bit integer containing the swapped bytes.
> + */
> +static __inline__ int __attribute__((__always_inline__, __nodebug__))
> +__bswapd(int __A) {
> +  return __builtin_bswap32(__A);
> +}
> +
> +#define _bswap(A) __bswapd((A))
> +#define _bit_scan_forward(A) __bsfd((A))
> +#define _bit_scan_reverse(A) __bsrd((A))
> +
> +#ifdef __x86_64__
> +/** Find the first set bit starting from the lsb. Result is undefined if
> + *  input is 0.
> + *
> + *  \headerfile 
> + *
> + *  This intrinsic corresponds to the  BSF  instruction or the
> + *   TZCNT  instruction.
> + *
> + *  \param __A
> + * A 64-bit integer operand.
> + *  \returns A 32-bit integer containing the bit number.
> + */
> +static __inline__ int __attribute__((__always_inline__, __nodebug__))
> +__bsfq(long long __A) {
> +  return __builtin_ctzll(__A);
> +}
> +
> +/** Find the first set bit starting from the msb. Result is undefined if
> + *  input is 0.
> + *
> + *  \headerfile 
> + *
> + *  This intrinsic corresponds to the  BSR  instruction or the
> + *   LZCNT  instruction and an  XOR .
> + *
> + *  \param __A
> + * A 64-bit integer operand.
> + *  \returns A 32-bit integer containing the bit number.
> + */
> +static __inline__ int __attribute__((__always_inline__, __nodebug__))
> +__bsrq(long long __A) {
> +  return 63 - __builtin_clzll(__A);
> +}
> +
> +/** Swaps the bytes in the input. Converting little endian to big endian
> or
> + *  vice versa.
> + *
> + *  \headerfile 
> + *
> + *  This intrinsic corresponds to the  BSWAP  instruction.
> + *
> + *  \param __A
> + * A 64-bit integer operand.
> + *  \returns A 64-bit integer containing the swapped bytes.
> + */
> +static __inline__ long long __attribute__((__always_inline__,
> __nodebug__))
> +__bswapq(long long __A) {
> +  return __builtin_bswap64(__A);
> +}
> +
> +#define _bswap64(A) __bswapq((A))
> +#endif
> +
>  /** Counts the number of bits in the source operand having a value of 1.
>   *
>   *  \headerfile 
>
> Modified: cfe/trunk/lib/Headers/immintrin.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/immintrin.h?rev=356848&r1=356847&r2=356848&view=diff
>

[PATCH] D53757: [ASTImporter] Changed use of Import to Import_New in ASTNodeImporter.

2019-03-24 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.

Hi Balasz,

Sorry, I missed the review accidentally. Thank you for the patch!




Comment at: lib/AST/ASTImporter.cpp:3418
 
-  for (const auto *Attr : D->attrs())
-ToIndirectField->addAttr(Importer.Import(Attr));

shafik wrote:
> Why is this section of code removed?
I guess the reason is that this import is already done inside 
`GetImportedOrCreateDecl()`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53757



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


[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

2019-03-24 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment.

Hello Raphael,

Thank you for the explanation. I have +1 to Gabor's question to understand if 
this functionality can actually be added to the common ASTImporter.




Comment at: clang/include/clang/AST/ASTImporter.h:149
+/// decl on its own.
+virtual Expected ImportInternal(Decl *From);
+

I'd suggest to rename it to `ImportImpl`.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:590
+  new RedirectingImporter(ToContext, ToFileManager, FromContext,
+  FromFileManager, MinimalImport, LookupTabl));
+};

LookupTable?



Comment at: clang/unittests/AST/ASTImporterTest.cpp:578
+auto *ND = dyn_cast(FromD);
+if (!ND)
+  return ASTImporter::ImportInternal(FromD);

I think it's better to merge these conditions: `if (!ND || ND->getName() != 
"shouldNotBeImported")`



Comment at: clang/unittests/AST/ASTImporterTest.cpp:583
+for (Decl *D : getToContext().getTranslationUnitDecl()->decls()) {
+  if (auto ND = dyn_cast(D))
+if (ND->getName() == "realDecl")

`auto *` (pointer sign is needed).


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

https://reviews.llvm.org/D59485



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


[PATCH] D59692: [ASTImporter] Fix name conflict handling

2019-03-24 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment.

Hi Gabor,

Thank you for addressing the problem!




Comment at: lib/AST/ASTImporter.cpp:2256
 return Importer.MapImported(D, FoundTypedef);
-}
-// FIXME Handle redecl chain.
-break;
+} else
+  ConflictingDecls.push_back(FoundDecl);

`if` body is surrounded by braces, so it's better to surround `else` too.



Comment at: lib/AST/ASTImporter.cpp:2260
 
   ConflictingDecls.push_back(FoundDecl);
 }

Do we push the same decl twice?



Comment at: lib/AST/ASTImporter.cpp:8532
 unsigned NumDecls) {
-  return Name;
+  return DeclarationName();
 }

Empty DeclarationName can be valid sometimes. Should we return 
ErrorOr instead? This can also simplify caller code a bit.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59692



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


[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

2019-03-24 Thread Daniel Kolozsvari via Phabricator via cfe-commits
koldaniel added a comment.

In D35068#1438498 , @NoQ wrote:

> There seems to be a crash in this code. @koldaniel, would you like to take a 
> look? https://bugs.llvm.org/show_bug.cgi?id=41185


Hi,

True, it is a faulty scenario, my question is what should be the way forward? I 
think in case of built-in functions there should be no warning, since they 
differ from the deprecated ones which come from the old standard. The only 
purpose of the assert was to help development and maintenance (if a new 
function had been added, it should be decided if it is deprecated or unsafe). 
Returning instead of asserting would solve the problem.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D35068



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


[PATCH] D59746: [LibTooling] Fix '-h' option

2019-03-24 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment.

A better alternative would have been to add a cl::aliasopt for '-h' in llvm's 
CommandLineParser when '-help' was first added.  However, that's no longer 
possible since some llvm based tools already use '-h' for other purposes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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


[PATCH] D59746: [LibTooling] Fix '-h' option

2019-03-24 Thread Don Hinton via Phabricator via cfe-commits
hintonda created this revision.
hintonda added a reviewer: alexfh.
Herald added a project: clang.

LibTooling's '-h' option was intended to be as an alias for
'-help', but since the '-help' option is declared as a function scoped
static, it isn't visible, leaving it to individual clients to handle
'-h' -- which none seem to do.

This fix scans argv and expands '-h' to '-help' so that llvm's
CommandLineParser can handle it appropriately.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp


Index: clang/lib/Tooling/CommonOptionsParser.cpp
===
--- clang/lib/Tooling/CommonOptionsParser.cpp
+++ clang/lib/Tooling/CommonOptionsParser.cpp
@@ -114,8 +114,24 @@
   if (!ErrorMessage.empty())
 ErrorMessage.append("\n");
   llvm::raw_string_ostream OS(ErrorMessage);
+
+  // Expand "-h" to "-help" so it's static handler can be used.
+  SmallVector NewArgv;
+  // Append options from command line.
+  bool SawDoubleDash = false;
+  for (int I = 0; I < argc; ++I) {
+StringRef Arg(argv[I]);
+if (Arg == "--")
+  SawDoubleDash = true;
+if(!SawDoubleDash && Arg == "-h")
+  NewArgv.push_back("-help");
+else
+  NewArgv.push_back(argv[I]);
+  }
+  int NewArgc = static_cast(NewArgv.size());
+
   // Stop initializing if command-line option parsing failed.
-  if (!cl::ParseCommandLineOptions(argc, argv, Overview, &OS)) {
+  if (!cl::ParseCommandLineOptions(NewArgc, &NewArgv[0], Overview, &OS)) {
 OS.flush();
 return llvm::make_error("[CommonOptionsParser]: " +
ErrorMessage,


Index: clang/lib/Tooling/CommonOptionsParser.cpp
===
--- clang/lib/Tooling/CommonOptionsParser.cpp
+++ clang/lib/Tooling/CommonOptionsParser.cpp
@@ -114,8 +114,24 @@
   if (!ErrorMessage.empty())
 ErrorMessage.append("\n");
   llvm::raw_string_ostream OS(ErrorMessage);
+
+  // Expand "-h" to "-help" so it's static handler can be used.
+  SmallVector NewArgv;
+  // Append options from command line.
+  bool SawDoubleDash = false;
+  for (int I = 0; I < argc; ++I) {
+StringRef Arg(argv[I]);
+if (Arg == "--")
+  SawDoubleDash = true;
+if(!SawDoubleDash && Arg == "-h")
+  NewArgv.push_back("-help");
+else
+  NewArgv.push_back(argv[I]);
+  }
+  int NewArgc = static_cast(NewArgv.size());
+
   // Stop initializing if command-line option parsing failed.
-  if (!cl::ParseCommandLineOptions(argc, argv, Overview, &OS)) {
+  if (!cl::ParseCommandLineOptions(NewArgc, &NewArgv[0], Overview, &OS)) {
 OS.flush();
 return llvm::make_error("[CommonOptionsParser]: " +
ErrorMessage,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r356851 - [pp-trace] Delete redundant clang::

2019-03-24 Thread Fangrui Song via cfe-commits
Author: maskray
Date: Sun Mar 24 00:31:21 2019
New Revision: 356851

URL: http://llvm.org/viewvc/llvm-project?rev=356851&view=rev
Log:
[pp-trace] Delete redundant clang::

And clarify command line options

Modified:
clang-tools-extra/trunk/docs/pp-trace.rst
clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp
clang-tools-extra/trunk/pp-trace/PPTrace.cpp

Modified: clang-tools-extra/trunk/docs/pp-trace.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/pp-trace.rst?rev=356851&r1=356850&r2=356851&view=diff
==
--- clang-tools-extra/trunk/docs/pp-trace.rst (original)
+++ clang-tools-extra/trunk/docs/pp-trace.rst Sun Mar 24 00:31:21 2019
@@ -23,7 +23,7 @@ pp-trace Usage
 Command Line Format
 ---
 
-``pp-trace []  []``
+``pp-trace []  [-- ]``
 
  is a place-holder for options
 specific to pp-trace, which are described below in

Modified: clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp?rev=356851&r1=356850&r2=356851&view=diff
==
--- clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp (original)
+++ clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp Sun Mar 24 00:31:21 
2019
@@ -21,13 +21,13 @@ namespace clang {
 namespace pp_trace {
 
 // Get a "file:line:column" source location string.
-static std::string getSourceLocationString(clang::Preprocessor &PP,
-   clang::SourceLocation Loc) {
+static std::string getSourceLocationString(Preprocessor &PP,
+   SourceLocation Loc) {
   if (Loc.isInvalid())
 return std::string("(none)");
 
   if (Loc.isFileID()) {
-clang::PresumedLoc PLoc = PP.getSourceManager().getPresumedLoc(Loc);
+PresumedLoc PLoc = PP.getSourceManager().getPresumedLoc(Loc);
 
 if (PLoc.isInvalid()) {
   return std::string("(invalid)");
@@ -91,7 +91,7 @@ static const char *const MappingStrings[
 
 PPCallbacksTracker::PPCallbacksTracker(const FilterType &Filters,
std::vector 
&CallbackCalls,
-   clang::Preprocessor &PP)
+   Preprocessor &PP)
 : CallbackCalls(CallbackCalls), Filters(Filters), PP(PP) {}
 
 PPCallbacksTracker::~PPCallbacksTracker() {}
@@ -99,9 +99,10 @@ PPCallbacksTracker::~PPCallbacksTracker(
 // Callback functions.
 
 // Callback invoked whenever a source file is entered or exited.
-void PPCallbacksTracker::FileChanged(
-clang::SourceLocation Loc, clang::PPCallbacks::FileChangeReason Reason,
-clang::SrcMgr::CharacteristicKind FileType, clang::FileID PrevFID) {
+void PPCallbacksTracker::FileChanged(SourceLocation Loc,
+ PPCallbacks::FileChangeReason Reason,
+ SrcMgr::CharacteristicKind FileType,
+ FileID PrevFID) {
   beginCallback("FileChanged");
   appendArgument("Loc", Loc);
   appendArgument("Reason", Reason, FileChangeReasonStrings);
@@ -111,10 +112,9 @@ void PPCallbacksTracker::FileChanged(
 
 // Callback invoked whenever a source file is skipped as the result
 // of header guard optimization.
-void
-PPCallbacksTracker::FileSkipped(const clang::FileEntry &SkippedFile,
-const clang::Token &FilenameTok,
-clang::SrcMgr::CharacteristicKind FileType) {
+void PPCallbacksTracker::FileSkipped(const FileEntry &SkippedFile,
+ const Token &FilenameTok,
+ SrcMgr::CharacteristicKind FileType) {
   beginCallback("FileSkipped");
   appendArgument("ParentFile", &SkippedFile);
   appendArgument("FilenameTok", FilenameTok);
@@ -135,11 +135,10 @@ PPCallbacksTracker::FileNotFound(llvm::S
 // any kind (#include, #import, etc.) has been processed, regardless
 // of whether the inclusion will actually result in an inclusion.
 void PPCallbacksTracker::InclusionDirective(
-clang::SourceLocation HashLoc, const clang::Token &IncludeTok,
-llvm::StringRef FileName, bool IsAngled,
-clang::CharSourceRange FilenameRange, const clang::FileEntry *File,
+SourceLocation HashLoc, const Token &IncludeTok, llvm::StringRef FileName,
+bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,
 llvm::StringRef SearchPath, llvm::StringRef RelativePath,
-const clang::Module *Imported, clang::SrcMgr::CharacteristicKind FileType) 
{
+const Module *Imported, SrcMgr::CharacteristicKind FileType) {
   beginCallback("InclusionDirective");
   appendArgument("IncludeTok", IncludeTok);
   appendFilePathArgument("FileName", FileName);
@@ -153,9 +152,9 @@ void PPCallbacksTracker::InclusionDirect
 
 // Callback invoked 

[clang-tools-extra] r356850 - [pp-trace] Wrap code in clang::pp_trace

2019-03-24 Thread Fangrui Song via cfe-commits
Author: maskray
Date: Sun Mar 24 00:21:32 2019
New Revision: 356850

URL: http://llvm.org/viewvc/llvm-project?rev=356850&view=rev
Log:
[pp-trace] Wrap code in clang::pp_trace

Modified:
clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp
clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.h
clang-tools-extra/trunk/pp-trace/PPTrace.cpp

Modified: clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp?rev=356850&r1=356849&r2=356850&view=diff
==
--- clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp (original)
+++ clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp Sun Mar 24 00:21:32 
2019
@@ -17,7 +17,8 @@
 #include "clang/Lex/MacroArgs.h"
 #include "llvm/Support/raw_ostream.h"
 
-// Utility functions.
+namespace clang {
+namespace pp_trace {
 
 // Get a "file:line:column" source location string.
 static std::string getSourceLocationString(clang::Preprocessor &PP,
@@ -455,7 +456,7 @@ void PPCallbacksTracker::appendArgument(
 void PPCallbacksTracker::appendArgument(const char *Name, const char *Value) {
   if (DisableTrace)
 return;
-  CallbackCalls.back().Arguments.push_back(Argument(Name, Value));
+  CallbackCalls.back().Arguments.push_back(Argument{Name, Value});
 }
 
 // Append a string object argument to the top trace item.
@@ -674,3 +675,6 @@ PPCallbacksTracker::getSourceString(clan
   const char *E = PP.getSourceManager().getCharacterData(Range.getEnd());
   return llvm::StringRef(B, E - B);
 }
+
+} // namespace pp_trace
+} // namespace clang

Modified: clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.h?rev=356850&r1=356849&r2=356850&view=diff
==
--- clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.h (original)
+++ clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.h Sun Mar 24 00:21:32 
2019
@@ -32,14 +32,11 @@
 #include 
 #include 
 
-/// \brief This class represents one callback function argument by name
-///   and value.
-class Argument {
-public:
-  Argument(llvm::StringRef Name, llvm::StringRef Value)
-  : Name(Name), Value(Value) {}
-  Argument() = default;
+namespace clang {
+namespace pp_trace {
 
+// This struct represents one callback function argument by name and value.
+struct Argument {
   std::string Name;
   std::string Value;
 };
@@ -74,7 +71,7 @@ using FilterType = std::vector &CallbackCalls,
- clang::Preprocessor &PP);
+ Preprocessor &PP);
 
   ~PPCallbacksTracker() override;
 
   // Overidden callback functions.
 
-  void FileChanged(clang::SourceLocation Loc,
-   clang::PPCallbacks::FileChangeReason Reason,
-   clang::SrcMgr::CharacteristicKind FileType,
-   clang::FileID PrevFID = clang::FileID()) override;
-  void FileSkipped(const clang::FileEntry &SkippedFile,
-   const clang::Token &FilenameTok,
-   clang::SrcMgr::CharacteristicKind FileType) override;
+  void FileChanged(SourceLocation Loc, PPCallbacks::FileChangeReason Reason,
+   SrcMgr::CharacteristicKind FileType,
+   FileID PrevFID = FileID()) override;
+  void FileSkipped(const FileEntry &SkippedFile, const Token &FilenameTok,
+   SrcMgr::CharacteristicKind FileType) override;
   bool FileNotFound(llvm::StringRef FileName,
 llvm::SmallVectorImpl &RecoveryPath) override;
-  void InclusionDirective(clang::SourceLocation HashLoc,
-  const clang::Token &IncludeTok,
+  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
   llvm::StringRef FileName, bool IsAngled,
-  clang::CharSourceRange FilenameRange,
-  const clang::FileEntry *File,
+  CharSourceRange FilenameRange, const FileEntry *File,
   llvm::StringRef SearchPath,
-  llvm::StringRef RelativePath,
-  const clang::Module *Imported,
-  clang::SrcMgr::CharacteristicKind FileType) override;
-  void moduleImport(clang::SourceLocation ImportLoc, clang::ModuleIdPath Path,
-const clang::Module *Imported) override;
+  llvm::StringRef RelativePath, const Module *Imported,
+  SrcMgr::CharacteristicKind FileType) override;
+  void moduleImport(SourceLocation ImportLoc, ModuleIdPath Path,
+const Module *Imported) override;
   void EndOfMainFile() override;
-  void Ident(clang::SourceLocation Loc, llvm::StringRef str) override;
-  void PragmaDirective(clang::SourceLocation Loc,
-