[PATCH] D121602: [clang][dataflow] Model the behavior of non-standard optional constructors

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev updated this revision to Diff 415349.
sgatev marked 3 inline comments as done.
sgatev added a comment.

Address reviewers' comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121602

Files:
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -31,8 +31,8 @@
 
 // FIXME: Move header definitions in separate file(s).
 static constexpr char StdTypeTraitsHeader[] = R"(
-#ifndef TYPE_TRAITS_H
-#define TYPE_TRAITS_H
+#ifndef STD_TYPE_TRAITS_H
+#define STD_TYPE_TRAITS_H
 
 namespace std {
 
@@ -127,6 +127,9 @@
   typedef T type;
 };
 
+template 
+using remove_cv_t = typename remove_cv::type;
+
 template 
 struct decay {
  private:
@@ -139,9 +142,196 @@
typename remove_cv::type>::type>::type type;
 };
 
+template 
+struct enable_if {};
+
+template 
+struct enable_if {
+  typedef T type;
+};
+
+template 
+using enable_if_t = typename enable_if::type;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : true_type {};
+
+template 
+struct is_void : is_same::type> {};
+
+namespace detail {
+
+template 
+auto try_add_rvalue_reference(int) -> type_identity;
+template 
+auto try_add_rvalue_reference(...) -> type_identity;
+
+}  // namespace detail
+
+template 
+struct add_rvalue_reference : decltype(detail::try_add_rvalue_reference(0)) {
+};
+
+template 
+typename add_rvalue_reference::type declval() noexcept;
+
+namespace detail {
+
+template 
+auto test_returnable(int)
+-> decltype(void(static_cast(nullptr)), true_type{});
+template 
+auto test_returnable(...) -> false_type;
+
+template 
+auto test_implicitly_convertible(int)
+-> decltype(void(declval()(declval())), true_type{});
+template 
+auto test_implicitly_convertible(...) -> false_type;
+
+}  // namespace detail
+
+template 
+struct is_convertible
+: integral_constant(0))::value &&
+ decltype(detail::test_implicitly_convertible(
+ 0))::value) ||
+(is_void::value && is_void::value)> {};
+
+template 
+inline constexpr bool is_convertible_v = is_convertible::value;
+
+template 
+using void_t = void;
+
+template 
+struct is_constructible_ : false_type {};
+
+template 
+struct is_constructible_()...))>, T, Args...>
+: true_type {};
+
+template 
+using is_constructible = is_constructible_, T, Args...>;
+
+template 
+inline constexpr bool is_constructible_v = is_constructible::value;
+
+template 
+struct __uncvref {
+  typedef typename remove_cv::type>::type type;
+};
+
+template 
+using __uncvref_t = typename __uncvref<_Tp>::type;
+
+template 
+using _BoolConstant = integral_constant;
+
+template 
+using _IsSame = _BoolConstant<__is_same(_Tp, _Up)>;
+
+template 
+using _IsNotSame = _BoolConstant;
+
+template 
+struct _MetaBase;
+template <>
+struct _MetaBase {
+  template 
+  using _SelectImpl = _Tp;
+  template  class _FirstFn, template  class,
+class... _Args>
+  using _SelectApplyImpl = _FirstFn<_Args...>;
+  template 
+  using _FirstImpl = _First;
+  template 
+  using _SecondImpl = _Second;
+  template 
+  using _OrImpl =
+  typename _MetaBase<_First::value != true && sizeof...(_Rest) != 0>::
+  template _OrImpl<_First, _Rest...>;
+};
+
+template <>
+struct _MetaBase {
+  template 
+  using _SelectImpl = _Up;
+  template  class, template  class _SecondFn,
+class... _Args>
+  using _SelectApplyImpl = _SecondFn<_Args...>;
+  template 
+  using _OrImpl = _Result;
+};
+
+template 
+using _If = typename _MetaBase<_Cond>::template _SelectImpl<_IfRes, _ElseRes>;
+
+template 
+using _Or = typename _MetaBase::template _OrImpl;
+
+template 
+using __enable_if_t = typename enable_if<_Bp, _Tp>::type;
+
+template 
+using __expand_to_true = true_type;
+template 
+__expand_to_true<__enable_if_t<_Pred::value>...> __and_helper(int);
+template 
+false_type __and_helper(...);
+template 
+using _And = decltype(__and_helper<_Pred...>(0));
+
+struct __check_tuple_constructor_fail {
+  static constexpr bool __enable_explicit_default() { return false; }
+  static constexpr bool __enable_implicit_default() { return false; }
+  template 
+  static constexpr bool __enable_explicit() {
+return false;
+  }
+  template 
+  static constexpr bool __enable_implicit() {
+return false;
+  }
+};
+
 } // namespace std
 
-#endif // TYPE_TRAITS_

[PATCH] D121602: [clang][dataflow] Model the behavior of non-standard optional constructors

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h:146
+  std::vector>
+  Actions;

xazax.hun wrote:
> Nit: looks like we need to repeat the action type. Should we restore the 
> using above?
It's not the same. The one above is a template - it accepts an arbitrary `Node` 
instead of a `Stmt`.



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:118
+/// function will be 2.
+int countOptionalWrappers(const ASTContext &ASTCtx, QualType Type) {
+  if (!IsOptionalType(Type))

xazax.hun wrote:
> Nit: some of these functions are static and some not. Should we have one big 
> anonymous namespace instead?
We already have an anonymous namespace so I'll make the functions inside 
non-static.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121602

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

[clang] 092a530 - [clang][dataflow] Model the behavior of non-standard optional constructors

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-03-15T08:13:13Z
New Revision: 092a530ca1878df08dc616cb43072044a39fb132

URL: 
https://github.com/llvm/llvm-project/commit/092a530ca1878df08dc616cb43072044a39fb132
DIFF: 
https://github.com/llvm/llvm-project/commit/092a530ca1878df08dc616cb43072044a39fb132.diff

LOG: [clang][dataflow] Model the behavior of non-standard optional constructors

Model nullopt, inplace, value, and conversion constructors.

Reviewed-by: ymandel, xazax.hun, gribozavr2

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h 
b/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
index b319360627911..2aaaf78f9cd0e 100644
--- a/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
+++ b/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
@@ -70,28 +70,24 @@ using MatchSwitch = std::function;
 /// \endcode
 template  class MatchSwitchBuilder {
 public:
-  // An action is triggered by the match of a pattern against the input
-  // statement. For generality, actions take both the matched statement and the
-  // set of bindings produced by the match.
-  using Action = std::function;
-
-  MatchSwitchBuilder &&CaseOf(ast_matchers::internal::Matcher M,
-  Action A) && {
-Matchers.push_back(std::move(M));
-Actions.push_back(std::move(A));
-return std::move(*this);
-  }
-
-  // Convenience function for the common case, where bound nodes are not
-  // needed. `Node` should be a subclass of `Stmt`.
+  /// Registers an action that will be triggered by the match of a pattern
+  /// against the input statement.
+  ///
+  /// Requirements:
+  ///
+  ///  `Node` should be a subclass of `Stmt`.
   template 
-  MatchSwitchBuilder &&CaseOf(ast_matchers::internal::Matcher M,
-  void (*Action)(const Node *, State &)) && {
+  MatchSwitchBuilder &&
+  CaseOf(ast_matchers::internal::Matcher M,
+ std::function
+ A) && {
 Matchers.push_back(std::move(M));
-Actions.push_back([Action](const Stmt *Stmt,
-   const ast_matchers::MatchFinder::MatchResult &,
-   State &S) { Action(cast(Stmt), S); });
+Actions.push_back(
+[A = std::move(A)](const Stmt *Stmt,
+   const ast_matchers::MatchFinder::MatchResult &R,
+   State &S) { A(cast(Stmt), R, S); });
 return std::move(*this);
   }
 
@@ -146,7 +142,9 @@ template  class MatchSwitchBuilder {
   }
 
   std::vector Matchers;
-  std::vector Actions;
+  std::vector>
+  Actions;
 };
 } // namespace dataflow
 } // namespace clang

diff  --git 
a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp 
b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 0963e8e452448..171b22b7e9130 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -22,7 +22,7 @@ using namespace ::clang::ast_matchers;
 
 using LatticeTransferState = TransferState;
 
-static auto optionalClass() {
+auto optionalClass() {
   return classTemplateSpecializationDecl(
   anyOf(hasName("std::optional"), hasName("std::__optional_storage_base"),
 hasName("__optional_destruct_base"), hasName("absl::optional"),
@@ -30,26 +30,54 @@ static auto optionalClass() {
   hasTemplateArgument(0, refersToType(type().bind("T";
 }
 
-static auto hasOptionalType() { return hasType(optionalClass()); }
+auto hasOptionalType() { return hasType(optionalClass()); }
 
-static auto isOptionalMemberCallWithName(llvm::StringRef MemberName) {
+auto isOptionalMemberCallWithName(llvm::StringRef MemberName) {
   return cxxMemberCallExpr(
   on(expr(unless(cxxThisExpr(,
   callee(cxxMethodDecl(hasName(MemberName), ofClass(optionalClass();
 }
 
-static auto isOptionalOperatorCallWithName(llvm::StringRef OperatorName) {
+auto isOptionalOperatorCallWithName(llvm::StringRef OperatorName) {
   return cxxOperatorCallExpr(hasOverloadedOperatorName(OperatorName),
  callee(cxxMethodDecl(ofClass(optionalClass();
 }
 
-static auto isMakeOptionalCall() {
+auto isMakeOptionalCall() {
   return callExpr(
   callee(functionDecl(hasAnyName(
   "std::make_optional", "base::make_optional", 
"absl::make_optional"))),
   hasOptionalType());
 }
 
+auto hasNulloptType() {
+  return hasType(name

[PATCH] D121602: [clang][dataflow] Model the behavior of non-standard optional constructors

[Stanislav Gatev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG092a530ca187: [clang][dataflow] Model the behavior of 
non-standard optional constructors (authored by sgatev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121602

Files:
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -31,8 +31,8 @@
 
 // FIXME: Move header definitions in separate file(s).
 static constexpr char StdTypeTraitsHeader[] = R"(
-#ifndef TYPE_TRAITS_H
-#define TYPE_TRAITS_H
+#ifndef STD_TYPE_TRAITS_H
+#define STD_TYPE_TRAITS_H
 
 namespace std {
 
@@ -127,6 +127,9 @@
   typedef T type;
 };
 
+template 
+using remove_cv_t = typename remove_cv::type;
+
 template 
 struct decay {
  private:
@@ -139,9 +142,196 @@
typename remove_cv::type>::type>::type type;
 };
 
+template 
+struct enable_if {};
+
+template 
+struct enable_if {
+  typedef T type;
+};
+
+template 
+using enable_if_t = typename enable_if::type;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : true_type {};
+
+template 
+struct is_void : is_same::type> {};
+
+namespace detail {
+
+template 
+auto try_add_rvalue_reference(int) -> type_identity;
+template 
+auto try_add_rvalue_reference(...) -> type_identity;
+
+}  // namespace detail
+
+template 
+struct add_rvalue_reference : decltype(detail::try_add_rvalue_reference(0)) {
+};
+
+template 
+typename add_rvalue_reference::type declval() noexcept;
+
+namespace detail {
+
+template 
+auto test_returnable(int)
+-> decltype(void(static_cast(nullptr)), true_type{});
+template 
+auto test_returnable(...) -> false_type;
+
+template 
+auto test_implicitly_convertible(int)
+-> decltype(void(declval()(declval())), true_type{});
+template 
+auto test_implicitly_convertible(...) -> false_type;
+
+}  // namespace detail
+
+template 
+struct is_convertible
+: integral_constant(0))::value &&
+ decltype(detail::test_implicitly_convertible(
+ 0))::value) ||
+(is_void::value && is_void::value)> {};
+
+template 
+inline constexpr bool is_convertible_v = is_convertible::value;
+
+template 
+using void_t = void;
+
+template 
+struct is_constructible_ : false_type {};
+
+template 
+struct is_constructible_()...))>, T, Args...>
+: true_type {};
+
+template 
+using is_constructible = is_constructible_, T, Args...>;
+
+template 
+inline constexpr bool is_constructible_v = is_constructible::value;
+
+template 
+struct __uncvref {
+  typedef typename remove_cv::type>::type type;
+};
+
+template 
+using __uncvref_t = typename __uncvref<_Tp>::type;
+
+template 
+using _BoolConstant = integral_constant;
+
+template 
+using _IsSame = _BoolConstant<__is_same(_Tp, _Up)>;
+
+template 
+using _IsNotSame = _BoolConstant;
+
+template 
+struct _MetaBase;
+template <>
+struct _MetaBase {
+  template 
+  using _SelectImpl = _Tp;
+  template  class _FirstFn, template  class,
+class... _Args>
+  using _SelectApplyImpl = _FirstFn<_Args...>;
+  template 
+  using _FirstImpl = _First;
+  template 
+  using _SecondImpl = _Second;
+  template 
+  using _OrImpl =
+  typename _MetaBase<_First::value != true && sizeof...(_Rest) != 0>::
+  template _OrImpl<_First, _Rest...>;
+};
+
+template <>
+struct _MetaBase {
+  template 
+  using _SelectImpl = _Up;
+  template  class, template  class _SecondFn,
+class... _Args>
+  using _SelectApplyImpl = _SecondFn<_Args...>;
+  template 
+  using _OrImpl = _Result;
+};
+
+template 
+using _If = typename _MetaBase<_Cond>::template _SelectImpl<_IfRes, _ElseRes>;
+
+template 
+using _Or = typename _MetaBase::template _OrImpl;
+
+template 
+using __enable_if_t = typename enable_if<_Bp, _Tp>::type;
+
+template 
+using __expand_to_true = true_type;
+template 
+__expand_to_true<__enable_if_t<_Pred::value>...> __and_helper(int);
+template 
+false_type __and_helper(...);
+template 
+using _And = decltype(__and_helper<_Pred...>(0));
+
+struct __check_tuple_constructor_fail {
+  static constexpr bool __enable_explicit_default() { return false; }
+  static constexpr bool __enable_implicit_default() { return false; }
+  template 
+  static constexpr bool __enable_explicit() {
+return false;
+  }
+  template 
+ 

[PATCH] D111400: [Clang] Implement P2242R3

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 415353.
cor3ntin added a comment.

Fix test messed up by automatic formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaCXX/constant-expression-cxx2b.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1361,7 +1361,7 @@
 
   Non-literal variables (and labels and gotos) in constexpr functions
   https://wg21.link/P2242R3";>P2242R3
-  No
+  Clang 15
 
 
   Character encoding of diagnostic text
Index: clang/test/SemaCXX/constant-expression-cxx2b.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constant-expression-cxx2b.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify %s -fcxx-exceptions -triple=x86_64-linux-gnu -Wpre-c++2b-compat
+
+constexpr int f(int n) {  // expected-error {{constexpr function never produces a constant expression}}
+  static const int m = n; // expected-warning {{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+  // expected-note {{control flows through the declaration of a static variable}}
+  return m;
+}
+constexpr int g(int n) {//  expected-error {{constexpr function never produces a constant expression}}
+  thread_local const int m = n; //  expected-warning {{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+// expected-note {{control flows through the declaration of a thread_local variable}}
+  return m;
+}
+
+constexpr int h(int n) {  // expected-error {{constexpr function never produces a constant expression}}
+  static const int m = n; // expected-warning {{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+  // expected-note {{control flows through the declaration of a static variable}}
+  return &m - &m;
+}
+constexpr int i(int n) {//  expected-error {{constexpr function never produces a constant expression}}
+  thread_local const int m = n; //  expected-warning {{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+// expected-note {{control flows through the declaration of a thread_local variable}}
+  return &m - &m;
+}
+
+constexpr int j(int n) {
+  if (!n)
+return 0;
+  static const int m = n; // expected-warning{{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}}
+  return m;
+}
+
+constexpr int k(int n) {
+  if (!n)
+return 0;
+  thread_local const int m = n; // expected-warning{{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+
+  return m;
+}
+
+constexpr int j_evaluated(int n) {
+  if (!n)
+return 0;
+  static const int m = n; // expected-warning{{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+  // expected-note {{control flows through the declaration of a static variable}}
+  return m;
+}
+
+constexpr int je = j_evaluated(1); // expected-error {{constexpr variable 'je' must be initialized by a constant expression}}  \
+   // expected-note {{in call}}
+
+constexpr int k_evaluated(int n) {
+  if (!n)
+return 0;
+  thread_local const int m = n; // expected-warning{{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+// expected-note {{control flows through the declaration of a thread_local variable}}
+
+  return m;
+}
+
+constexpr int ke = k_evaluated(1); //expected-error {{constexpr variable 'ke' must be initialized by a constant expression}} \
+   //expected-note {{in call}}
+
+namespace eval_goto {
+
+constexpr int f(int x) {
+  if (x) {
+return 0;
+  } else {
+goto test; // expected-note {{subexpression not valid in a constant expression}} \
+// expected-warning {{use of this statement in a constexp

[PATCH] D120527: [OpaquePtr][AArch64] Use elementtype on ldxr/stxr

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

@aeubanks Do you plan to take care of the corresponding arm intrinsics as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120527

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

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

I am still concerned that there is an expectation that. the fcxx-modules option 
is connected with clang modules.
.. see, for example:

https://github.com/llvm/llvm-project/blob/d90d45fc9029cc7dbb6d44798f51131df6b2eef1/clang/lib/Driver/ToolChains/Clang.cpp#L3579

and

https://github.com/llvm/llvm-project/blob/875782bd9ea322445dd41213ca6bbf70169e741d/clang/include/clang/Driver/Options.td#L5549

(those are the two places that the option currently appears in the code; test 
cases that have the option seem to be connected with clang/implicit modules).

So I will defer to other folks to comment further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120540

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

[PATCH] D121576: [clang-format] Don't unwrap lines preceded by line comments

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Please do, to be honest I spent enough time on this and I got to the same 
conclusion, it needs a rethink. Please go ahead, I don't use this feature 
myself so I'm not likely to see all the issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121576

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

[PATCH] D121588: [C++20][Modules][Driver][HU 1/N] Initial handling for -xc++-{system,user}-header.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

note: I do not plan to fix the formatting issue in clang/lib/Driver/Types.cpp, 
since I am adding one line and the format change would mean ≈ 110 lines of 
changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121588

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

[PATCH] D121678: [pseudo] Split greatergreater token.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang.

For a >> token (a right shift operator, or a nested template?), the clang
lexer always returns a single greatergreater token, as a result,
the grammar-based GLR parser never try to parse the nested template
case.

We derive a token stream by always splitting the >> token, so that the
GLR parser is able to pursue both options during parsing (usually 1
path fails).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121678

Files:
  clang/include/clang/Tooling/Syntax/Pseudo/Token.h
  clang/lib/Tooling/Syntax/Pseudo/Token.cpp
  clang/lib/Tooling/Syntax/Pseudo/cxx.bnf
  clang/unittests/Tooling/Syntax/Pseudo/TokenTest.cpp

Index: clang/unittests/Tooling/Syntax/Pseudo/TokenTest.cpp
===
--- clang/unittests/Tooling/Syntax/Pseudo/TokenTest.cpp
+++ clang/unittests/Tooling/Syntax/Pseudo/TokenTest.cpp
@@ -172,6 +172,30 @@
 }));
 }
 
+TEST(TokenTest, SplitGreaterGreater) {
+  LangOptions Opts;
+  std::string Code = R"cpp(
+>> // split
+// >> with an escaped newline in the middle, split
+>\
+>
+>>= // not split
+)cpp";
+  TokenStream Raw = stripComments(cook(lex(Code, Opts), Opts));
+  EXPECT_THAT(Raw.tokens(),
+  ElementsAreArray({token(">>", tok::greatergreater),
+token(">>", tok::greatergreater),
+token(">>=", tok::greatergreaterequal)}));
+  TokenStream Split = splitGreaterGreater(Raw);
+  EXPECT_THAT(Split.tokens(), ElementsAreArray({
+  token(">", tok::greater),
+  token(">", tok::greater),
+  token(">", tok::greater),
+  token(">", tok::greater),
+  token(">>=", tok::greatergreaterequal),
+  }));
+}
+
 TEST(TokenTest, DropComments) {
   LangOptions Opts;
   std::string Code = R"cpp(
Index: clang/lib/Tooling/Syntax/Pseudo/cxx.bnf
===
--- clang/lib/Tooling/Syntax/Pseudo/cxx.bnf
+++ clang/lib/Tooling/Syntax/Pseudo/cxx.bnf
@@ -13,6 +13,9 @@
 #  - the file merely describes the core C++ grammar. Preprocessor directives and
 #lexical conversions are omitted as we reuse clang's lexer and run a fake
 #preprocessor;
+#  - grammar rules with the >> token are adjusted, the greatergreater token is
+#split into two > tokens, to make the GLR parser aware of nested templates
+#and right shift operator.
 #
 # Guidelines:
 #   - non-terminals are lower_case; terminals (aka tokens) correspond to
@@ -96,7 +99,8 @@
 fold-operator := ^
 fold-operator := |
 fold-operator := <<
-fold-operator := >>
+#! Custom modification to split >> to two greater token.
+fold-operator := > >
 fold-operator := +=
 fold-operator := -=
 fold-operator := *=
@@ -202,7 +206,8 @@
 # expr.shift
 shift-expression := additive-expression
 shift-expression := shift-expression << additive-expression
-shift-expression := shift-expression >> additive-expression
+#! Custom modification to split >> to two greater token.
+shift-expression := shift-expression > > additive-expression
 # expr.spaceship
 compare-expression := shift-expression
 compare-expression := compare-expression <=> shift-expression
@@ -615,7 +620,8 @@
 operator-name := ^^
 operator-name := ||
 operator-name := <<
-operator-name := >>
+#! Custom modification to split >> to two greater token.
+operator-name := > >
 operator-name := <<=
 operator-name := >>=
 operator-name := ++
Index: clang/lib/Tooling/Syntax/Pseudo/Token.cpp
===
--- clang/lib/Tooling/Syntax/Pseudo/Token.cpp
+++ clang/lib/Tooling/Syntax/Pseudo/Token.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Tooling/Syntax/Pseudo/Token.h"
+#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -93,6 +94,29 @@
 OS << '\n';
 }
 
+TokenStream splitGreaterGreater(const TokenStream &Input) {
+  TokenStream Out;
+  for (Token T : Input.tokens()) {
+// FIXME: split lessless token to support Cuda's triple angle brackets <<<.
+if (T.Kind == tok::greatergreater) {
+  if (T.text() == ">>") {
+T.Kind = tok::greater;
+T.Length = 1;
+Out.push(T);
+T.Data = T.text().data() + 1;
+// FIXME: Line is wrong if the first greater is followed by an escaped
+// newline.
+Out.push(T);
+continue;
+  }
+  assert(false && "split an uncook token stream!");
+}
+Out.push(std::move(T));
+  }
+  Out.finalize(

[PATCH] D121327: Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO

[Orlando Cazalet-Hyams via Phabricator via cfe-commits]

Orlando added a comment.

Hi @yln, the test you added 
`llvm/test/Transforms/LowerGlobalDestructors/lower-global-dtors.ll` is failing 
on some bots, e.g.  https://lab.llvm.org/buildbot/#/builders/139/builds/18527 
(builder `llvm-clang-x86_64-sie-ubuntu-fast`). Please can you take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121327

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

[PATCH] D121589: [C++20][Modules][Driver][HU 2/N] Add fmodule-header, fmodule-header=

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D121589#3381343 , @ChuanqiXu wrote:

>> It's not practical to recognise a header without any suffix so
>
> -fmodule-header=system foo isn't going to happen.
>
> May I ask the reason? It looks not so good with `-fmodule-header=system 
> -xc++-header vector`

OK. One should never say "never" ;) , it would be nicer if we could avoid this.

... it would require a policy change in the driver - since we cannot recognise 
files like 'vector' as headers, they are currently unclaimed (which means that 
they default to being considered as linker inputs).

This is a long-standing (forever, I suspect) situation;
Although we could make it so that if we see certain options, all unknown inputs 
get claimed as source files (or headers) I wonder how much build system code 
that might break.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121589

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

[PATCH] D121593: [clangd][WIP] Provide clang-include-cleaner

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This is exciting! We weren't sure how much effort to put into decoupling this 
from clangd, whether there'd be interest in such a standalone tool. Some 
thoughts at a high level:

---

Eventually we should separate this from clangd. I don't think it necessarily 
needs to be right away, but we should avoid hopelessly entangling it.
In particular, I don't think we should be going through `ClangdServer`. As well 
as pulling in ~all of clangd, this is going to spawn threads, write PCHes to 
disk, and limit flexibility in how we process files.

---

> I'm not sure why include-cleaner wasn't integrated in clang-tidy, but I 
> suspect there's a good reason.

The TL;DR is I think it could be, and we should consider whether to do this 
instead of adding a separate tool.

Our initial goal was to get this functionality in clangd. While clangd has 
support for tidy checks, it's limited: the checks only see AST and PP events 
from the main file (for performance reasons).
This means if you implemented a clang-tidy check in clangd in the obvious way 
(collect `#include` information in PPCallbacks, query it while traversing) this 
check would not work in clangd.
Instead, we built it into clangd in a way that's aware of our preamble 
optimization - most of the PP events are reused across reparses.

However the capture-PP-info part is fairly simple, and the 
traverse-AST-and-gather-locations part is independent of clangd.
We could "easily" share this part as a library and build a clang-tidy check out 
of the same core.

Advantages to making this a clang-tidy check:

- for users: fits into existing workflows and integrations
- for maintainers: existing framework takes care of parsing ASTs, emitting 
diagnostics, applying fixes

Advantages to a separate tool:

- more control over how code is processed, e.g. traversing headers exactly once
- the check abstraction may not be a perfect fit, e.g. maybe produce a report 
instead of diagnostics, or treatment of alternate fixes
- probably some minor efficiency gains as we're not really using matchers etc

Overall I suspect making this a tidy check would make it more useful to more 
people. It would mean sorting out the dependency issue sooner rather than later 
though.

---

I think it's important to have some spelled-out scope of what we want this tool 
to do and what workflows it's aiming for.

- applying fixes vs reporting problems? (If applying, what to do when multiple 
fixes are possible)
- adding vs removing includes, or both? (We do plan to support adding includes 
in clangd)
- being conservative vs being complete (feeling safe to run this unattended is 
a big feature, c.f. IWYU)
- do you run it over files or codebases? (important to consider how headers are 
treated)
- assume "well-behaved" code, or does it try to understand everything?
- does it make use of forward declarations, or just includes?

It's OK to make some of these configurable, but the answers are important to 
default behavior and also where effort is focused.

Hopefully many of the answers are aligned with what we want for clangd. If not 
that's OK, but we should plan for this rather than be surprised when it happens 
:-)




Comment at: clang-tools-extra/clangd/CMakeLists.txt:183
 add_subdirectory(tool)
+add_subdirectory(include-cleaner)
 add_subdirectory(indexer)

kbobyrev wrote:
> I think it would be better to just put it into `tool/`.
LLVM's CMake rules make it hard to have multiple binaries in the same 
directory, so I think this is correct as-is.



Comment at: clang-tools-extra/clangd/include-cleaner/CMakeLists.txt:1
+add_clang_tool(clang-include-cleaner
+  ClangIncludeCleanerMain.cpp

Until this is a bit more ready for real use, I think this should be 
`add_clang_executable` so it doesn't get packaged


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121593

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

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Nvm. See https://github.com/llvm/llvm-project/issues/54384.




Comment at: clang/lib/Format/TokenAnnotator.cpp:216
 FormatToken &OpeningParen = *CurrentToken->Previous;
 assert(OpeningParen.is(tok::l_paren));
 FormatToken *PrevNonComment = OpeningParen.getPreviousNonComment();

owenpan wrote:
> This patch either uncovers or causes an assertion failure here on a bunch of 
> files in `clang/test`.
I was wrong! Please ignore it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121550

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

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

I agree. My understanding is that `-fcxx-modules` enables Clang modules that 
don't interact with C++20 modules. @Bigcheese, any thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120540

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

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D120540#3381905 , @jansvoboda11 
wrote:

> I agree. My understanding is that `-fcxx-modules` enables Clang modules that 
> don't interact with C++20 modules. @Bigcheese, any thoughts?

Oh, if it is true, then it is in a chaos really. I mean the language variables 
set in `-fcxx-modules` is `CPlusPlusModules`. And we uses `CPlusPlusModules ` 
as the induction variable for C++20 modules for a relative long time.

My observation is that:

- `CPlusPlusModules ` is true if we set `-std=c++20` before this patch.
- `Modules` is true if we set `-fmodules` or `-fcxx-modules`.
- What I get from the previous patch is that we hope a smooth change from clang 
modules to C++20 modules.

So my conclusion might be that the `-fcxx-modules` is connected to both `C++20 
modules` and `Clang Modules`. Is this intended or not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120540

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

[PATCH] D119701: [clangd] Re-enable clang-tidy's nolint blocks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.
Herald added a project: All.

thanks, sorry for missing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119701

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

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

xbolva00 added a comment.
Herald added a project: All.

In D120305#3347191 , @MaskRay wrote:

> In D120305#3347177 , @nikic wrote:
>
>> Yes, because you reverted the change for that one buildbot, of course it is 
>> green now. You could have also made the buildbot green by disabling tests on 
>> that bot. Or disabling sanitizers on it. Doesn't change the fact that the 
>> configuration it was originally testing is still broken, you just hid the 
>> failure.
>
>
>
> In D120305#3347184 , @tstellar 
> wrote:
>
>> In D120305#3347161 , @MaskRay 
>> wrote:
>>
>>> In D120305#3347160 , @nikic wrote:
>>>
 @MaskRay Please revert the change and all dependent changes you have made. 
 A revert is not a personal affront to you. It's not a judgement that you 
 or your change are bad. It's a simple matter of policy and standard 
 procedure. There's a good chance that next week you'll get confirmation 
 that it's indeed some outdated libraries on the buildbot, and the change 
 can reland without any changes on your side. Or maybe it turns out that 
 this default is not quite viable for powerpc targets yet. Who knows.

 Don't worry about the churn. These short-term reverts happen all the time, 
 we're used to it. Especially for tiny changes like this one, it's really 
 no problem (reverts can be more annoying if the commit touches 1500 test 
 files).
>>>
>>> I have mentioned that https://lab.llvm.org/buildbot/#/builders/57 has been 
>>> green.
>>
>> Disabling the failing tests with an unreviewed patch is not the right way to 
>> fix this.
>
> clang-ppc64le-rhel got `-DCLANG_DEFAULT_PIE_ON_LINUX=OFF` ~9 hours ago. If 
> any of you can make the configuration live on the bot, it will work and we 
> can re-enable the tests.

https://lab.llvm.org/buildbot/#/builders/57

Somebody (@tstellar) should remove these powerpc shitty bots, red almost all 
the time as bot maintainers do nothing. There should be clear policy when to 
remove bad, broken, unmaintained bots.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120305

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

[PATCH] D121589: [C++20][Modules][Driver][HU 2/N] Add fmodule-header, fmodule-header=

[Chuanqi Xu via Phabricator via cfe-commits]

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

In D121589#3381894 , @iains wrote:

> In D121589#3381343 , @ChuanqiXu 
> wrote:
>
>>> It's not practical to recognise a header without any suffix so
>>
>> -fmodule-header=system foo isn't going to happen.
>>
>> May I ask the reason? It looks not so good with `-fmodule-header=system 
>> -xc++-header vector`
>
> OK. One should never say "never" ;) , it would be nicer if we could avoid 
> this.
>
> ... it would require a policy change in the driver - since we cannot 
> recognise files like 'vector' as headers, they are currently unclaimed (which 
> means that they default to being considered as linker inputs).
>
> This is a long-standing (forever, I suspect) situation;
> Although we could make it so that if we see certain options, all unknown 
> inputs get claimed as source files (or headers) I wonder how much build 
> system code that might break.

If this is the policy, I am OK. For the build systems, due to the dependences, 
all build systems want to support C++20 modules should do works. So if we are 
doing our best, I think it might not be bad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121589

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

[clang] 126b37a - [clang-format] Correctly recognize arrays in template parameter list.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-03-15T11:33:13+01:00
New Revision: 126b37a713dc1c67cbc7dc8b5288b2f907c906a9

URL: 
https://github.com/llvm/llvm-project/commit/126b37a713dc1c67cbc7dc8b5288b2f907c906a9
DIFF: 
https://github.com/llvm/llvm-project/commit/126b37a713dc1c67cbc7dc8b5288b2f907c906a9.diff

LOG: [clang-format] Correctly recognize arrays in template parameter list.

Fixes https://github.com/llvm/llvm-project/issues/54245.

Reviewed By: MyDeveloperDay, HazardyKnusperkeks, owenpan

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index b03296c7e02d9..cadf1960dbf7a 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1935,6 +1935,11 @@ bool UnwrappedLineParser::tryToParseLambda() {
   if (!tryToParseLambdaIntroducer())
 return false;
 
+  // `[something] >` is not a lambda, but an array type in a template parameter
+  // list.
+  if (FormatTok->is(tok::greater))
+return false;
+
   bool SeenArrow = false;
   bool InTemplateParameterList = false;
 
@@ -3524,7 +3529,7 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
   // Don't try parsing a lambda if we had a closing parenthesis before,
   // it was probably a pointer to an array: int (*)[].
   if (!tryToParseLambda())
-break;
+continue;
 } else {
   parseSquare();
   continue;

diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index dff8c04662601..5f346fa5f8bfa 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -98,6 +98,20 @@ TEST_F(TokenAnnotatorTest, UnderstandsStructs) {
   auto Tokens = annotate("struct S {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("template  struct S {};");
+  EXPECT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::l_square, TT_ArraySubscriptLSquare);
+  EXPECT_TOKEN(Tokens[13], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("template  struct S {};");
+  EXPECT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::l_square, TT_ArraySubscriptLSquare);
+  EXPECT_TOKEN(Tokens[13], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_StructLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUnions) {



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

[PATCH] D121584: [clang-format] Correctly recognize arrays in template parameter list.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG126b37a713dc: [clang-format] Correctly recognize arrays in 
template parameter list. (authored by curdeius).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121584

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -98,6 +98,20 @@
   auto Tokens = annotate("struct S {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("template  struct S {};");
+  EXPECT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::l_square, TT_ArraySubscriptLSquare);
+  EXPECT_TOKEN(Tokens[13], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("template  struct S {};");
+  EXPECT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::l_square, TT_ArraySubscriptLSquare);
+  EXPECT_TOKEN(Tokens[13], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_StructLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUnions) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1935,6 +1935,11 @@
   if (!tryToParseLambdaIntroducer())
 return false;
 
+  // `[something] >` is not a lambda, but an array type in a template parameter
+  // list.
+  if (FormatTok->is(tok::greater))
+return false;
+
   bool SeenArrow = false;
   bool InTemplateParameterList = false;
 
@@ -3524,7 +3529,7 @@
   // Don't try parsing a lambda if we had a closing parenthesis before,
   // it was probably a pointer to an array: int (*)[].
   if (!tryToParseLambda())
-break;
+continue;
 } else {
   parseSquare();
   continue;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -98,6 +98,20 @@
   auto Tokens = annotate("struct S {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("template  struct S {};");
+  EXPECT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::l_square, TT_ArraySubscriptLSquare);
+  EXPECT_TOKEN(Tokens[13], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("template  struct S {};");
+  EXPECT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::l_square, TT_ArraySubscriptLSquare);
+  EXPECT_TOKEN(Tokens[13], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_StructLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUnions) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1935,6 +1935,11 @@
   if (!tryToParseLambdaIntroducer())
 return false;
 
+  // `[something] >` is not a lambda, but an array type in a template parameter
+  // list.
+  if (FormatTok->is(tok::greater))
+return false;
+
   bool SeenArrow = false;
   bool InTemplateParameterList = false;
 
@@ -3524,7 +3529,7 @@
   // Don't try parsing a lambda if we had a closing parenthesis before,
   // it was probably a pointer to an array: int (*)[].
   if (!tryToParseLambda())
-break;
+continue;
 } else {
   parseSquare();
   continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121682: [clang-format] Fix crashes due to missing l_paren

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: HazardyKnusperkeks, curdeius, MyDeveloperDay.
owenpan added a project: clang-format.
Herald added a project: All.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/54384.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121682

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12089,6 +12089,7 @@
   verifyFormat("if {\n  foo;\n  foo();\n}");
   verifyFormat("switch {\n  foo;\n  foo();\n}");
   verifyIncompleteFormat("for {\n  foo;\n  foo();\n}");
+  verifyIncompleteFormat("ERROR: for target;");
   verifyFormat("while {\n  foo;\n  foo();\n}");
   verifyFormat("do {\n  foo;\n  foo();\n} while;");
 }
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1023,6 +1023,8 @@
   if (Style.isCpp() && CurrentToken && CurrentToken->is(tok::kw_co_await))
 next();
   Contexts.back().ColonIsForRangeExpr = true;
+  if (!CurrentToken || CurrentToken->isNot(tok::l_paren))
+return false;
   next();
   if (!parseParens())
 return false;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12089,6 +12089,7 @@
   verifyFormat("if {\n  foo;\n  foo();\n}");
   verifyFormat("switch {\n  foo;\n  foo();\n}");
   verifyIncompleteFormat("for {\n  foo;\n  foo();\n}");
+  verifyIncompleteFormat("ERROR: for target;");
   verifyFormat("while {\n  foo;\n  foo();\n}");
   verifyFormat("do {\n  foo;\n  foo();\n} while;");
 }
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1023,6 +1023,8 @@
   if (Style.isCpp() && CurrentToken && CurrentToken->is(tok::kw_co_await))
 next();
   Contexts.back().ColonIsForRangeExpr = true;
+  if (!CurrentToken || CurrentToken->isNot(tok::l_paren))
+return false;
   next();
   if (!parseParens())
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121588: [C++20][Modules][Driver][HU 1/N] Initial handling for -xc++-{system,user}-header.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a subscriber: MyDeveloperDay.
ChuanqiXu added a comment.

In D121588#3381872 , @iains wrote:

> note: I do not plan to fix the formatting issue in 
> clang/lib/Driver/Types.cpp, since I am adding one line and the format change 
> would mean ≈ 110 lines of changes.

It is odd. I thought all things could be handled by `clang-format-diff`. 
@MyDeveloperDay could you help to take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121588

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

[PATCH] D121678: [pseudo] Split greatergreater token.

[Haojian Wu via Phabricator via cfe-commits]

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

define a greatergreater nonterminal per discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121678

Files:
  clang/include/clang/Tooling/Syntax/Pseudo/Token.h
  clang/lib/Tooling/Syntax/Pseudo/Token.cpp
  clang/lib/Tooling/Syntax/Pseudo/cxx.bnf
  clang/unittests/Tooling/Syntax/Pseudo/TokenTest.cpp

Index: clang/unittests/Tooling/Syntax/Pseudo/TokenTest.cpp
===
--- clang/unittests/Tooling/Syntax/Pseudo/TokenTest.cpp
+++ clang/unittests/Tooling/Syntax/Pseudo/TokenTest.cpp
@@ -172,6 +172,30 @@
 }));
 }
 
+TEST(TokenTest, SplitGreaterGreater) {
+  LangOptions Opts;
+  std::string Code = R"cpp(
+>> // split
+// >> with an escaped newline in the middle, split
+>\
+>
+>>= // not split
+)cpp";
+  TokenStream Raw = stripComments(cook(lex(Code, Opts), Opts));
+  EXPECT_THAT(Raw.tokens(),
+  ElementsAreArray({token(">>", tok::greatergreater),
+token(">>", tok::greatergreater),
+token(">>=", tok::greatergreaterequal)}));
+  TokenStream Split = splitGreaterGreater(Raw);
+  EXPECT_THAT(Split.tokens(), ElementsAreArray({
+  token(">", tok::greater),
+  token(">", tok::greater),
+  token(">", tok::greater),
+  token(">", tok::greater),
+  token(">>=", tok::greatergreaterequal),
+  }));
+}
+
 TEST(TokenTest, DropComments) {
   LangOptions Opts;
   std::string Code = R"cpp(
Index: clang/lib/Tooling/Syntax/Pseudo/cxx.bnf
===
--- clang/lib/Tooling/Syntax/Pseudo/cxx.bnf
+++ clang/lib/Tooling/Syntax/Pseudo/cxx.bnf
@@ -13,6 +13,9 @@
 #  - the file merely describes the core C++ grammar. Preprocessor directives and
 #lexical conversions are omitted as we reuse clang's lexer and run a fake
 #preprocessor;
+#  - grammar rules with the >> token are adjusted, the greatergreater token is
+#split into two > tokens, to make the GLR parser aware of nested templates
+#and right shift operator.
 #
 # Guidelines:
 #   - non-terminals are lower_case; terminals (aka tokens) correspond to
@@ -96,7 +99,7 @@
 fold-operator := ^
 fold-operator := |
 fold-operator := <<
-fold-operator := >>
+fold-operator := greatergreater
 fold-operator := +=
 fold-operator := -=
 fold-operator := *=
@@ -202,7 +205,7 @@
 # expr.shift
 shift-expression := additive-expression
 shift-expression := shift-expression << additive-expression
-shift-expression := shift-expression >> additive-expression
+shift-expression := shift-expression greatergreater additive-expression
 # expr.spaceship
 compare-expression := shift-expression
 compare-expression := compare-expression <=> shift-expression
@@ -615,7 +618,7 @@
 operator-name := ^^
 operator-name := ||
 operator-name := <<
-operator-name := >>
+operator-name := greatergreater
 operator-name := <<=
 operator-name := >>=
 operator-name := ++
@@ -737,3 +740,8 @@
 module-keyword := IDENTIFIER
 import-keyword := IDENTIFIER
 export-keyword := IDENTIFIER
+
+#! Greatergreater token -- clang lexer always lexes it as a single token, we
+#! split it into two tokens to make the GLR parser aware of the nested-template
+#! case.
+greatergreater := > >
Index: clang/lib/Tooling/Syntax/Pseudo/Token.cpp
===
--- clang/lib/Tooling/Syntax/Pseudo/Token.cpp
+++ clang/lib/Tooling/Syntax/Pseudo/Token.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Tooling/Syntax/Pseudo/Token.h"
+#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -93,6 +94,29 @@
 OS << '\n';
 }
 
+TokenStream splitGreaterGreater(const TokenStream &Input) {
+  TokenStream Out;
+  for (Token T : Input.tokens()) {
+// FIXME: split lessless token to support Cuda's triple angle brackets <<<.
+if (T.Kind == tok::greatergreater) {
+  if (T.text() == ">>") {
+T.Kind = tok::greater;
+T.Length = 1;
+Out.push(T);
+T.Data = T.text().data() + 1;
+// FIXME: Line is wrong if the first greater is followed by an escaped
+// newline.
+Out.push(T);
+continue;
+  }
+  assert(false && "split an uncook token stream!");
+}
+Out.push(std::move(T));
+  }
+  Out.finalize();
+  return Out;
+}
+
 TokenStream stripComments(const TokenStream &Input) {
   TokenStream Out;
   for (const Token &T : Input.tokens()) {
Index: clang/include/clang/Tooling/Syntax/Pseudo/Token.h
==

[PATCH] D121678: [pseudo] Split greatergreater token.

[Sam McCall via Phabricator via cfe-commits]

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

This seems like the right approach to me.

It seems a bit strange to be doing this whether we're parsing C++ or not, but I 
guess we're not going to want low-level grammar differences between C and C++ 
if we can avoid it.




Comment at: clang/lib/Tooling/Syntax/Pseudo/Token.cpp:97
 
+TokenStream splitGreaterGreater(const TokenStream &Input) {
+  TokenStream Out;

What do you think about combining this into cook()?
Seems a bit wasteful to do a separate pass for this. (We can always split it 
out again later if cook() gets too complicated)



Comment at: clang/lib/Tooling/Syntax/Pseudo/Token.cpp:107
+T.Data = T.text().data() + 1;
+// FIXME: Line is wrong if the first greater is followed by an escaped
+// newline.

I don't think we'll ever fix this, I'd drop the FIXME and add a `!` at the end 
:-)



Comment at: clang/lib/Tooling/Syntax/Pseudo/Token.cpp:112
+  }
+  assert(false && "split an uncook token stream!");
+}

why not assert at the top and remove the if?

This can only be caused by a really bad programmer error, I don't think we 
should worry about defending against it making it into production.



Comment at: clang/lib/Tooling/Syntax/Pseudo/cxx.bnf:102
 fold-operator := <<
-fold-operator := >>
+#! Custom modification to split >> to two greater token.
+fold-operator := > >

we should rather indirect this change through a new nonterminal:

```
greater-greater := > >
fold-operator := greater-greater
```

This lets us document the change in one place, and makes it easier to avoid the 
false parses if they prove important. We could restrict the first rule to only 
match if the second token has a `mergeable` bit or something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121678

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

[PATCH] D121533: [clang][deps] Fix traversal of precompiled dependencies

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 415380.
jansvoboda11 added a comment.

Throw away recursive implementation, add reproducer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121533

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/test/ClangScanDeps/modules-pch-dangling.c

Index: clang/test/ClangScanDeps/modules-pch-dangling.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-pch-dangling.c
@@ -0,0 +1,139 @@
+// Unsupported on AIX because we don't support the requisite "__clangast"
+// section in XCOFF yet.
+// UNSUPPORTED: aix
+
+// This test checks that the dependency scanner can handle larger amount of
+// explicitly built modules retrieved from the PCH.
+// (Previously, there was a bug dangling iterator bug that manifested only with
+// 16 and more retrieved modules.)
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- mod_00.h
+//--- mod_01.h
+//--- mod_02.h
+//--- mod_03.h
+//--- mod_04.h
+//--- mod_05.h
+//--- mod_06.h
+//--- mod_07.h
+//--- mod_08.h
+//--- mod_09.h
+//--- mod_10.h
+//--- mod_11.h
+//--- mod_12.h
+//--- mod_13.h
+//--- mod_14.h
+//--- mod_15.h
+//--- mod_16.h
+//--- mod.h
+#include "mod_00.h"
+#include "mod_01.h"
+#include "mod_02.h"
+#include "mod_03.h"
+#include "mod_04.h"
+#include "mod_05.h"
+#include "mod_06.h"
+#include "mod_07.h"
+#include "mod_08.h"
+#include "mod_09.h"
+#include "mod_10.h"
+#include "mod_11.h"
+#include "mod_12.h"
+#include "mod_13.h"
+#include "mod_14.h"
+#include "mod_15.h"
+#include "mod_16.h"
+//--- module.modulemap
+module mod_00 { header "mod_00.h" }
+module mod_01 { header "mod_01.h" }
+module mod_02 { header "mod_02.h" }
+module mod_03 { header "mod_03.h" }
+module mod_04 { header "mod_04.h" }
+module mod_05 { header "mod_05.h" }
+module mod_06 { header "mod_06.h" }
+module mod_07 { header "mod_07.h" }
+module mod_08 { header "mod_08.h" }
+module mod_09 { header "mod_09.h" }
+module mod_10 { header "mod_10.h" }
+module mod_11 { header "mod_11.h" }
+module mod_12 { header "mod_12.h" }
+module mod_13 { header "mod_13.h" }
+module mod_14 { header "mod_14.h" }
+module mod_15 { header "mod_15.h" }
+module mod_16 { header "mod_16.h" }
+module mod{ header "mod.h"}
+
+//--- pch.h
+#include "mod.h"
+
+//--- tu.c
+
+//--- cdb_pch.json.template
+[{
+  "file": "DIR/pch.h",
+  "directory": "DIR",
+  "command": "clang -x c-header DIR/pch.h -fmodules -gmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -o DIR/pch.h.gch"
+}]
+
+//--- cdb_tu.json.template
+[{
+  "file": "DIR/tu.c",
+  "directory": "DIR",
+  "command": "clang -fsyntax-only DIR/tu.c -fmodules -gmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -include DIR/pch.h -o DIR/tu.o"
+}]
+
+// Scan dependencies of the PCH:
+//
+// RUN: sed "s|DIR|%/t|g" %t/cdb_pch.json.template > %t/cdb_pch.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_pch.json -format experimental-full \
+// RUN:   -generate-modules-path-args -module-files-dir %t/build > %t/result_pch.json
+
+// Explicitly build the PCH:
+//
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_00 > %t/mod_00.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_01 > %t/mod_01.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_02 > %t/mod_02.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_03 > %t/mod_03.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_04 > %t/mod_04.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_05 > %t/mod_05.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_06 > %t/mod_06.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_07 > %t/mod_07.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_08 > %t/mod_08.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_09 > %t/mod_09.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_10 > %t/mod_10.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_11 > %t/mod_11.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_12 > %t/mod_12.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_13 > %t/mod_13.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_14 > %t/mod_14.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json --module-name=mod_15 > %t/mod

[PATCH] D121533: [clang][deps] Fix traversal of precompiled dependencies

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Fair enough, iterative implementation will be better.

I simplified it a bit by using the in-out parameter `ModuleFiles` to keep track 
of visited files. I also switched to using `SmallVector` for the newly 
discovered (not-yet-visited) imports, which allows using the suggested 
`pop_back_val` and avoids using (potentially dangling) iterator.

The test case is a bit unwieldy, since the old implementation only failed when 
the `StringMap` got rehashed (with 16 entries).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121533

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

[PATCH] D121295: [clang][deps] Modules don't contribute to search path usage

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 415381.
jansvoboda11 added a comment.

Undo unrelated changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121295

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Preprocessor/search-path-usage.m


Index: clang/test/Preprocessor/search-path-usage.m
===
--- clang/test/Preprocessor/search-path-usage.m
+++ clang/test/Preprocessor/search-path-usage.m
@@ -129,7 +129,7 @@
 #endif
 #endif
 
-// Check that search paths with module maps are reported.
+// Check that search paths with module maps are NOT reported.
 //
 // RUN: mkdir %t/modulemap_abs
 // RUN: sed "s|DIR|%/S/Inputs/search-path-usage|g"\
@@ -142,5 +142,5 @@
 // RUN:   -DMODMAP_ABS -verify
 #ifdef MODMAP_ABS
 @import b; // \
-// expected-remark-re {{search path used: '{{.*}}/modulemap_abs'}}
+// expected-no-diagnostics
 #endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -365,9 +365,6 @@
   break;
   }
 
-  if (Module)
-noteLookupUsage(It.Idx, ImportLoc);
-
   return Module;
 }
 


Index: clang/test/Preprocessor/search-path-usage.m
===
--- clang/test/Preprocessor/search-path-usage.m
+++ clang/test/Preprocessor/search-path-usage.m
@@ -129,7 +129,7 @@
 #endif
 #endif
 
-// Check that search paths with module maps are reported.
+// Check that search paths with module maps are NOT reported.
 //
 // RUN: mkdir %t/modulemap_abs
 // RUN: sed "s|DIR|%/S/Inputs/search-path-usage|g"\
@@ -142,5 +142,5 @@
 // RUN:   -DMODMAP_ABS -verify
 #ifdef MODMAP_ABS
 @import b; // \
-// expected-remark-re {{search path used: '{{.*}}/modulemap_abs'}}
+// expected-no-diagnostics
 #endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -365,9 +365,6 @@
   break;
   }
 
-  if (Module)
-noteLookupUsage(It.Idx, ImportLoc);
-
   return Module;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121685: [clang][deps] NFC: Use range-based for loop instead of iterators

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The iterator is needed after the loop body, meaning we can use more terse 
range-based for loop.

Depends on D121295 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121685

Files:
  clang/lib/Lex/HeaderSearch.cpp


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -299,20 +299,19 @@
SourceLocation ImportLoc,
bool AllowExtraModuleMapSearch) {
   Module *Module = nullptr;
-  SearchDirIterator It = nullptr;
 
   // Look through the various header search paths to load any available module
   // maps, searching for a module map that describes this module.
-  for (It = search_dir_begin(); It != search_dir_end(); ++It) {
-if (It->isFramework()) {
+  for (DirectoryLookup Dir : search_dir_range()) {
+if (Dir.isFramework()) {
   // Search for or infer a module map for a framework. Here we use
   // SearchName rather than ModuleName, to permit finding private modules
   // named FooPrivate in buggy frameworks named Foo.
   SmallString<128> FrameworkDirName;
-  FrameworkDirName += It->getFrameworkDir()->getName();
+  FrameworkDirName += Dir.getFrameworkDir()->getName();
   llvm::sys::path::append(FrameworkDirName, SearchName + ".framework");
   if (auto FrameworkDir = FileMgr.getDirectory(FrameworkDirName)) {
-bool IsSystem = It->getDirCharacteristic() != SrcMgr::C_User;
+bool IsSystem = Dir.getDirCharacteristic() != SrcMgr::C_User;
 Module = loadFrameworkModule(ModuleName, *FrameworkDir, IsSystem);
 if (Module)
   break;
@@ -322,12 +321,12 @@
 // FIXME: Figure out how header maps and module maps will work together.
 
 // Only deal with normal search directories.
-if (!It->isNormalDir())
+if (!Dir.isNormalDir())
   continue;
 
-bool IsSystem = It->isSystemHeaderDirectory();
+bool IsSystem = Dir.isSystemHeaderDirectory();
 // Search for a module map file in this directory.
-if (loadModuleMapFile(It->getDir(), IsSystem,
+if (loadModuleMapFile(Dir.getDir(), IsSystem,
   /*IsFramework*/false) == LMM_NewlyLoaded) {
   // We just loaded a module map file; check whether the module is
   // available now.
@@ -339,7 +338,7 @@
 // Search for a module map in a subdirectory with the same name as the
 // module.
 SmallString<128> NestedModuleMapDirName;
-NestedModuleMapDirName = It->getDir()->getName();
+NestedModuleMapDirName = Dir.getDir()->getName();
 llvm::sys::path::append(NestedModuleMapDirName, ModuleName);
 if (loadModuleMapFile(NestedModuleMapDirName, IsSystem,
   /*IsFramework*/false) == LMM_NewlyLoaded){
@@ -351,13 +350,13 @@
 
 // If we've already performed the exhaustive search for module maps in this
 // search directory, don't do it again.
-if (It->haveSearchedAllModuleMaps())
+if (Dir.haveSearchedAllModuleMaps())
   continue;
 
 // Load all module maps in the immediate subdirectories of this search
 // directory if ModuleName was from @import.
 if (AllowExtraModuleMapSearch)
-  loadSubdirectoryModuleMaps(*It);
+  loadSubdirectoryModuleMaps(Dir);
 
 // Look again for the module.
 Module = ModMap.findModule(ModuleName);


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -299,20 +299,19 @@
SourceLocation ImportLoc,
bool AllowExtraModuleMapSearch) {
   Module *Module = nullptr;
-  SearchDirIterator It = nullptr;
 
   // Look through the various header search paths to load any available module
   // maps, searching for a module map that describes this module.
-  for (It = search_dir_begin(); It != search_dir_end(); ++It) {
-if (It->isFramework()) {
+  for (DirectoryLookup Dir : search_dir_range()) {
+if (Dir.isFramework()) {
   // Search for or infer a module map for a framework. Here we use
   // SearchName rather than ModuleName, to permit finding private modules
   // named FooPrivate in buggy frameworks named Foo.
   SmallString<128> FrameworkDirName;
-  FrameworkDirName += It->getFrameworkDir()->getName();
+  FrameworkDirName += Dir.getFrameworkDir()->getName();
   llvm::sys::path::append(FrameworkDirName, SearchName + ".framework");
   if (auto FrameworkDir = FileMgr.getDirectory(FrameworkDirName)) {
-bool IsSystem = It->getDirCharacteri

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

I'm not sure it is exactly chaos, but it is certainly fragile and somewhat 
hard(er than necessary) to maintain.

We (@ChuanqiXu  and I at least) agree that there should be some way to make 
"which modules mode" unambiguous in both the driver and the compiler (I think 
we're only debating how/which flag to use)

The end game objective is that "when the compiler is processing for C++20 or 
later, then it should default to C++20+ modules". 
IMO, knowing "which modules mode" is in force does not alter this - if anything 
it ought to simplify checking whether we have completed migration (whatever 
that ends up meaning).

Presumably, we are never going to delete the other options (e.g. -fmodules etc) 
they would (perhaps) become deprecated and then "do nothing". OTOH, perhaps 
that mode will be needed by existing users for some considerable time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120540

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

[PATCH] D120952: [clang][AST matchers] adding submatchers under cudaKernelCallExpr to match kernel launch config

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D120952#3381224 , 
@ajohnson-uoregon wrote:

> I still need to write tests but: I do have a use case for these over here: 
> https://github.com/ajohnson-uoregon/llvm-project/blob/feature-ajohnson/clang-tools-extra/clang-rewrite/ConstructMatchers.cpp#L472
>  
> tl;dr, we'd like to match the kernel launch arguments (i.e., the arguments to 
> `__cudaPushCallConfiguration()`) and these matchers made writing the code to 
> generate those AST matchers much easier.

Thanks for the example use case, that helps! Do you expect to need it in more 
than one project though? We support defining local AST matchers so people can 
do one-off matching, but we typically only add AST matchers for things that we 
expect to be generally useful (multiple projects would benefit from it). For 
example, this project adds a number of local matchers: 
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp#L435

> Without at least the `hasKernelConfig()` matcher, it's actually currently 
> impossible to match the kernel launch args. (I wasn't able to find a way 
> after quite a while poking at the AST, at least.) As for the others, it's not 
> clear how to match the kernel launch args without exposing the fact that 
> there's a second CallExpr inside the CUDAKernelCallExpr to the user and 
> writing a pretty messy matcher, along the lines of 
> `cudaKernelCallExpr(hasKernelConfig(callExpr(hasArgument(0, expr()` for 
> the grid dim. `cudaKernelCallExpr(cudaGridDim())` is a lot cleaner and easier 
> to understand.

Yup, I think you need these kind of matchers for what you want to do. What I'm 
less certain of is whether others will need them (we don't have any 
CUDA-specific clang-tidy modules, I don't think any of the existing coding 
standards we support checks for have anything to say about CUDA, etc).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120952

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

[PATCH] D121687: [clang-tidy] Don't try to build CTTestTidyModule for Windows with dylibs

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: rnk.
Herald added subscribers: xazax.hun, mgorny.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: clang-tools-extra.

In MinGW mode, it's possible to build LLVM/Clang with
LLVM_LINK_LLVM_DYLIB (which implicitly enables plugins too). Other
existing ways of building plugins on Windows is to build with
LLVM_EXPORT_SYMBOLS_FOR_PLUGINS, where each executable exports its
symbols.

With LLVM_LINK_LLVM_DYLIB, we can't generally skip building plugins
even if they are set up with PLUGIN_TOOL, as some plugins (e.g.
under clang/examples) set up that way do build properly (as
they manually call clang_target_link_libraries, which links in the
libclang-cpp.dll dylib).

For CTTestTidyModule, there's no corresponding dylib that would
provide the same exports.

Alternatively, we could make the condition
`if (NOT WIN32 OR LLVM_EXPORT_SYMBOLS_FOR_PLUGINS)`, as that's the
only way this plugin would be possible to link on Windows.

(Currently, building this plugin with `LLVM_EXPORT_SYMBOLS_FOR_PLUGINS`
fails to link one symbol though, but in principle, it could
work.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121687

Files:
  clang-tools-extra/test/CMakeLists.txt


Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -81,11 +81,13 @@
 endforeach()
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
-  llvm_add_library(
-  CTTestTidyModule
-  MODULE clang-tidy/CTTestTidyModule.cpp
-  PLUGIN_TOOL clang-tidy
-  DEPENDS clang-tidy-headers)
+  if (NOT WIN32 AND NOT LLVM_LINK_LLVM_DYLIB)
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+  endif()
 
   if(CLANG_BUILT_STANDALONE)
 # LLVMHello library is needed below


Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -81,11 +81,13 @@
 endforeach()
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
-  llvm_add_library(
-  CTTestTidyModule
-  MODULE clang-tidy/CTTestTidyModule.cpp
-  PLUGIN_TOOL clang-tidy
-  DEPENDS clang-tidy-headers)
+  if (NOT WIN32 AND NOT LLVM_LINK_LLVM_DYLIB)
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+  endif()
 
   if(CLANG_BUILT_STANDALONE)
 # LLVMHello library is needed below
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121678: [pseudo] Split greatergreater token.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 415387.
hokein marked 4 inline comments as done.
hokein added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121678

Files:
  clang/include/clang/Tooling/Syntax/Pseudo/Token.h
  clang/lib/Tooling/Syntax/Pseudo/Lex.cpp
  clang/lib/Tooling/Syntax/Pseudo/Token.cpp
  clang/lib/Tooling/Syntax/Pseudo/cxx.bnf
  clang/unittests/Tooling/Syntax/Pseudo/TokenTest.cpp

Index: clang/unittests/Tooling/Syntax/Pseudo/TokenTest.cpp
===
--- clang/unittests/Tooling/Syntax/Pseudo/TokenTest.cpp
+++ clang/unittests/Tooling/Syntax/Pseudo/TokenTest.cpp
@@ -172,6 +172,25 @@
 }));
 }
 
+TEST(TokenTest, SplitGreaterGreater) {
+  LangOptions Opts;
+  std::string Code = R"cpp(
+>> // split
+// >> with an escaped newline in the middle, split
+>\
+>
+>>= // not split
+)cpp";
+  TokenStream Split = stripComments(cook(lex(Code, Opts), Opts));
+  EXPECT_THAT(Split.tokens(), ElementsAreArray({
+  token(">", tok::greater),
+  token(">", tok::greater),
+  token(">", tok::greater),
+  token(">", tok::greater),
+  token(">>=", tok::greatergreaterequal),
+  }));
+}
+
 TEST(TokenTest, DropComments) {
   LangOptions Opts;
   std::string Code = R"cpp(
Index: clang/lib/Tooling/Syntax/Pseudo/cxx.bnf
===
--- clang/lib/Tooling/Syntax/Pseudo/cxx.bnf
+++ clang/lib/Tooling/Syntax/Pseudo/cxx.bnf
@@ -13,6 +13,9 @@
 #  - the file merely describes the core C++ grammar. Preprocessor directives and
 #lexical conversions are omitted as we reuse clang's lexer and run a fake
 #preprocessor;
+#  - grammar rules with the >> token are adjusted, the greatergreater token is
+#split into two > tokens, to make the GLR parser aware of nested templates
+#and right shift operator.
 #
 # Guidelines:
 #   - non-terminals are lower_case; terminals (aka tokens) correspond to
@@ -96,7 +99,7 @@
 fold-operator := ^
 fold-operator := |
 fold-operator := <<
-fold-operator := >>
+fold-operator := greatergreater
 fold-operator := +=
 fold-operator := -=
 fold-operator := *=
@@ -202,7 +205,7 @@
 # expr.shift
 shift-expression := additive-expression
 shift-expression := shift-expression << additive-expression
-shift-expression := shift-expression >> additive-expression
+shift-expression := shift-expression greatergreater additive-expression
 # expr.spaceship
 compare-expression := shift-expression
 compare-expression := compare-expression <=> shift-expression
@@ -615,7 +618,7 @@
 operator-name := ^^
 operator-name := ||
 operator-name := <<
-operator-name := >>
+operator-name := greatergreater
 operator-name := <<=
 operator-name := >>=
 operator-name := ++
@@ -737,3 +740,8 @@
 module-keyword := IDENTIFIER
 import-keyword := IDENTIFIER
 export-keyword := IDENTIFIER
+
+#! greatergreater token -- clang lexer always lexes it as a single token, we
+#! split it into two tokens to make the GLR parser aware of the nested-template
+#! case.
+greatergreater := > >
Index: clang/lib/Tooling/Syntax/Pseudo/Token.cpp
===
--- clang/lib/Tooling/Syntax/Pseudo/Token.cpp
+++ clang/lib/Tooling/Syntax/Pseudo/Token.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Tooling/Syntax/Pseudo/Token.h"
+#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
Index: clang/lib/Tooling/Syntax/Pseudo/Lex.cpp
===
--- clang/lib/Tooling/Syntax/Pseudo/Lex.cpp
+++ clang/lib/Tooling/Syntax/Pseudo/Lex.cpp
@@ -99,10 +99,21 @@
   Tok.Length = Text.size();
   Tok.Flags &= ~static_cast(LexFlags::NeedsCleaning);
 }
-// Cook raw_identifiers into identifier, keyword, etc.
-if (Tok.Kind == tok::raw_identifier)
+
+if (Tok.Kind == tok::raw_identifier) {
+  // Cook raw_identifiers into identifier, keyword, etc.
   Tok.Kind = Identifiers.get(Tok.text()).getTokenID();
-Result.push(std::move(Tok));
+} else if (Tok.Kind == tok::greatergreater) {
+  // Split the greatergreater token.
+  // FIXME: split lessless token to support Cuda triple angle brackets <<<.
+  assert(Tok.text() == ">>");
+  Tok.Kind = tok::greater;
+  Tok.Length = 1;
+  Result.push(Tok);
+  // Line is wrong if the first greater is followed by an escaped newline!
+  Tok.Data = Tok.text().data() + 1;
+}
+Result.push((Tok));
   }
 
   Result.finalize();
Index: clang/include/clang/Tooling/Syntax/Pseudo/Token.h
=

[clang] e60defb - [clang-format] Add regression tests for function ref qualifiers on operator definition. NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-03-15T12:58:08+01:00
New Revision: e60defb931cfc333d143f6000a6a65ae4dc106a2

URL: 
https://github.com/llvm/llvm-project/commit/e60defb931cfc333d143f6000a6a65ae4dc106a2
DIFF: 
https://github.com/llvm/llvm-project/commit/e60defb931cfc333d143f6000a6a65ae4dc106a2.diff

LOG: [clang-format] Add regression tests for function ref qualifiers on 
operator definition. NFC.

Fixes https://github.com/llvm/llvm-project/issues/54374.

Added: 


Modified: 
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 354b60e27a5c7..557438de70fe0 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -9889,6 +9889,14 @@ TEST_F(FormatTest, UnderstandsFunctionRefQualification) {
   verifyFormat("template \n"
"void F(T) && = delete;",
getGoogleStyle());
+  verifyFormat("template  void operator=(T) &;");
+  verifyFormat("template  void operator=(T) const &;");
+  verifyFormat("template  void operator=(T) &noexcept;");
+  verifyFormat("template  void operator=(T) & = default;");
+  verifyFormat("template  void operator=(T) &&;");
+  verifyFormat("template  void operator=(T) && = delete;");
+  verifyFormat("template  void operator=(T) & {}");
+  verifyFormat("template  void operator=(T) && {}");
 
   FormatStyle AlignLeft = getLLVMStyle();
   AlignLeft.PointerAlignment = FormatStyle::PAS_Left;
@@ -9909,6 +9917,14 @@ TEST_F(FormatTest, UnderstandsFunctionRefQualification) {
   verifyFormat("void Fn(T const volatile&&) const volatile&&;", AlignLeft);
   verifyFormat("void Fn(T const volatile&&) const volatile&& noexcept;",
AlignLeft);
+  verifyFormat("template  void operator=(T) &;", AlignLeft);
+  verifyFormat("template  void operator=(T) const&;", AlignLeft);
+  verifyFormat("template  void operator=(T) & noexcept;", 
AlignLeft);
+  verifyFormat("template  void operator=(T) & = default;", 
AlignLeft);
+  verifyFormat("template  void operator=(T) &&;", AlignLeft);
+  verifyFormat("template  void operator=(T) && = delete;", 
AlignLeft);
+  verifyFormat("template  void operator=(T) & {}", AlignLeft);
+  verifyFormat("template  void operator=(T) && {}", AlignLeft);
 
   FormatStyle AlignMiddle = getLLVMStyle();
   AlignMiddle.PointerAlignment = FormatStyle::PAS_Middle;
@@ -9930,6 +9946,14 @@ TEST_F(FormatTest, UnderstandsFunctionRefQualification) {
   verifyFormat("void Fn(T const volatile &&) const volatile &&;", AlignMiddle);
   verifyFormat("void Fn(T const volatile &&) const volatile && noexcept;",
AlignMiddle);
+  verifyFormat("template  void operator=(T) &;", AlignMiddle);
+  verifyFormat("template  void operator=(T) const &;", 
AlignMiddle);
+  verifyFormat("template  void operator=(T) & noexcept;", 
AlignMiddle);
+  verifyFormat("template  void operator=(T) & = default;", 
AlignMiddle);
+  verifyFormat("template  void operator=(T) &&;", AlignMiddle);
+  verifyFormat("template  void operator=(T) && = delete;", 
AlignMiddle);
+  verifyFormat("template  void operator=(T) & {}", AlignMiddle);
+  verifyFormat("template  void operator=(T) && {}", AlignMiddle);
 
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInCStyleCastParentheses = true;

diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 5f346fa5f8bfa..9240e812ba927 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -159,6 +159,24 @@ TEST_F(TokenAnnotatorTest, UnderstandsDelete) {
   EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_CastRParen);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsFunctionRefQualifiers) {
+  auto Tokens = annotate("void f() &;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("void operator=() &&;");
+  EXPECT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("template  void f() &;");
+  EXPECT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("template  void operator=() &;");
+  EXPECT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::amp, TT_PointerOrReference);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresClausesAndConcepts) {
   auto Tokens = annotate("template \n"
  "concept C = (Foo && Bar) && (Bar && Baz);");



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

[clang] 3227aa3 - [clang-format] Correctly format variable templates.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-03-15T13:16:56+01:00
New Revision: 3227aa3aa83440ff94a3b13a29623e03b05098f2

URL: 
https://github.com/llvm/llvm-project/commit/3227aa3aa83440ff94a3b13a29623e03b05098f2
DIFF: 
https://github.com/llvm/llvm-project/commit/3227aa3aa83440ff94a3b13a29623e03b05098f2.diff

LOG: [clang-format] Correctly format variable templates.

Fixes https://github.com/llvm/llvm-project/issues/54257.

Reviewed By: MyDeveloperDay, HazardyKnusperkeks, owenpan

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 0c760b5b2811d..f0a17af677016 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1526,8 +1526,36 @@ class AnnotatingParser {
   if (Current.getPrecedence() != prec::Assignment)
 return false;
 
-  if (Line.First->isOneOf(tok::kw_template, tok::kw_using, tok::kw_return))
+  if (Line.First->isOneOf(tok::kw_using, tok::kw_return))
 return false;
+  if (Line.First->is(tok::kw_template)) {
+assert(Current.Previous);
+if (Current.Previous->is(tok::kw_operator)) {
+  // `template ... operator=` cannot be an expression.
+  return false;
+}
+
+// `template` keyword can start a variable template.
+const FormatToken *Tok = Line.First->getNextNonComment();
+assert(Tok); // Current token is on the same line.
+if (Tok->isNot(TT_TemplateOpener)) {
+  // Explicit template instantiations do not have `<>`.
+  return false;
+}
+
+Tok = Tok->MatchingParen;
+if (!Tok)
+  return false;
+Tok = Tok->getNextNonComment();
+if (!Tok)
+  return false;
+
+if (Tok->isOneOf(tok::kw_class, tok::kw_enum, tok::kw_concept,
+ tok::kw_struct, tok::kw_using))
+  return false;
+
+return true;
+  }
 
   // Type aliases use `type X = ...;` in TypeScript and can be exported
   // using `export type ...`.

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 557438de70fe0..459cbf1c7681c 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -25657,6 +25657,12 @@ TEST_F(FormatTest, 
AlignArrayOfStructuresRightAlignmentNonSquare) {
Style);
 }
 
+TEST_F(FormatTest, FormatsVariableTemplates) {
+  verifyFormat("inline bool var = is_integral_v && is_signed_v;");
+  verifyFormat("template  "
+   "inline bool var = is_integral_v && is_signed_v;");
+}
+
 } // namespace
 } // namespace format
 } // namespace clang

diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 9240e812ba927..cc624d0760d12 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -131,6 +131,31 @@ TEST_F(TokenAnnotatorTest, UnderstandsEnums) {
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_EnumLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsDefaultedAndDeletedFunctions) {
+  auto Tokens = annotate("auto operator<=>(const T &) const & = default;");
+  EXPECT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("template  void F(T) && = delete;");
+  EXPECT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsVariables) {
+  auto Tokens =
+  annotate("inline bool var = is_integral_v && is_signed_v;");
+  EXPECT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::ampamp, TT_BinaryOperator);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsVariableTemplates) {
+  auto Tokens =
+  annotate("template  "
+   "inline bool var = is_integral_v && is_signed_v;");
+  EXPECT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsLBracesInMacroDefinition) {
   auto Tokens = annotate("#define BEGIN NS {");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
@@ -164,17 +189,17 @@ TEST_F(TokenAnnotatorTest, 
UnderstandsFunctionRefQualifiers) {
   EXPECT_EQ(Tokens.size(), 7u) << Tokens;
   EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
 
-  Tokens = annotate("void operator=() &&;");
-  EXPECT_EQ(Tokens.size(), 8u) << Tokens;
-  EXPECT_TOKEN(Tokens[5], tok::ampamp, TT_PointerOrReference);
+  Tokens = annotate("void operator=(T) &&;");
+  EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::ampamp, TT_PointerOrReference);
 
   Tokens = annotate("templa

[PATCH] D121456: [clang-format] Correctly format variable templates.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Changes 
Planned".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3227aa3aa834: [clang-format] Correctly format variable 
templates. (authored by curdeius).

Changed prior to commit:
  https://reviews.llvm.org/D121456?vs=414954&id=415390#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121456

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -131,6 +131,31 @@
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_EnumLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsDefaultedAndDeletedFunctions) {
+  auto Tokens = annotate("auto operator<=>(const T &) const & = default;");
+  EXPECT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("template  void F(T) && = delete;");
+  EXPECT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsVariables) {
+  auto Tokens =
+  annotate("inline bool var = is_integral_v && is_signed_v;");
+  EXPECT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::ampamp, TT_BinaryOperator);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsVariableTemplates) {
+  auto Tokens =
+  annotate("template  "
+   "inline bool var = is_integral_v && is_signed_v;");
+  EXPECT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsLBracesInMacroDefinition) {
   auto Tokens = annotate("#define BEGIN NS {");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
@@ -164,17 +189,17 @@
   EXPECT_EQ(Tokens.size(), 7u) << Tokens;
   EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
 
-  Tokens = annotate("void operator=() &&;");
-  EXPECT_EQ(Tokens.size(), 8u) << Tokens;
-  EXPECT_TOKEN(Tokens[5], tok::ampamp, TT_PointerOrReference);
+  Tokens = annotate("void operator=(T) &&;");
+  EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::ampamp, TT_PointerOrReference);
 
   Tokens = annotate("template  void f() &;");
   EXPECT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::amp, TT_PointerOrReference);
 
-  Tokens = annotate("template  void operator=() &;");
-  EXPECT_EQ(Tokens.size(), 13u) << Tokens;
-  EXPECT_TOKEN(Tokens[10], tok::amp, TT_PointerOrReference);
+  Tokens = annotate("template  void operator=(T) &;");
+  EXPECT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresClausesAndConcepts) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25657,6 +25657,12 @@
Style);
 }
 
+TEST_F(FormatTest, FormatsVariableTemplates) {
+  verifyFormat("inline bool var = is_integral_v && is_signed_v;");
+  verifyFormat("template  "
+   "inline bool var = is_integral_v && is_signed_v;");
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1526,8 +1526,36 @@
   if (Current.getPrecedence() != prec::Assignment)
 return false;
 
-  if (Line.First->isOneOf(tok::kw_template, tok::kw_using, tok::kw_return))
+  if (Line.First->isOneOf(tok::kw_using, tok::kw_return))
 return false;
+  if (Line.First->is(tok::kw_template)) {
+assert(Current.Previous);
+if (Current.Previous->is(tok::kw_operator)) {
+  // `template ... operator=` cannot be an expression.
+  return false;
+}
+
+// `template` keyword can start a variable template.
+const FormatToken *Tok = Line.First->getNextNonComment();
+assert(Tok); // Current token is on the same line.
+if (Tok->isNot(TT_TemplateOpener)) {
+  // Explicit template instantiations do not have `<>`.
+  return false;
+}
+
+Tok = Tok->MatchingParen;
+if (!Tok)
+  return false;
+Tok = Tok->getNextNonComment();
+if (!Tok)
+  return false;
+
+if (Tok->isOneOf(tok::kw_class, tok::kw_enum, tok::kw_concept,
+ tok::kw_struct, tok::kw_using))
+  return false;
+
+return true;
+  }
 
   // Type aliases use `type X =

[PATCH] D88905: [Clang] Allow "ext_vector_type" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 415392.
simoll added a comment.

Blocking of arithmetic on bool vectors extends to the case where only one 
operand is a bool vector (and the other is, eg, a vector of int). This was 
still caught before during sema because the integer vector operand would have 
been implicitly truncated down to bool, which always has been illegal.
This change results in a different error message for the mixed 
bool-and-non-bool arithmetic case and a more explicit check in the code ("`||` 
instead of `&&`").


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88905

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/Sema/ext_vector_casts.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-size-conditional.cpp
  clang/test/SemaOpenCL/ext_vectors.cl

Index: clang/test/SemaOpenCL/ext_vectors.cl
===
--- clang/test/SemaOpenCL/ext_vectors.cl
+++ clang/test/SemaOpenCL/ext_vectors.cl
@@ -5,6 +5,7 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++2021
 
 typedef float float4 __attribute__((ext_vector_type(4)));
+typedef __attribute__((ext_vector_type(8))) bool BoolVector; // expected-error {{invalid vector element type 'bool'}}
 
 void test_ext_vector_accessors(float4 V) {
   V = V.wzyx;
Index: clang/test/SemaCXX/vector-size-conditional.cpp
===
--- clang/test/SemaCXX/vector-size-conditional.cpp
+++ clang/test/SemaCXX/vector-size-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((ext_vector_type(8)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
   all_dependent(four_ints, four_uints, two_uints); // expected-note {{in instantiation of}}
   all_dependent(four_ints, four_uints, four_uints);
 }
Index: clang/test/SemaCXX/vector-bool.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/vector-b

[PATCH] D121646: [Concepts] Fix an assertion failure while diagnosing constrained function candidates

[Erich Keane via Phabricator via cfe-commits]

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

In D121646#3380902 , @royjacobson 
wrote:

> In D121646#3380893 , @erichkeane 
> wrote:
>
>> This seems acceptable to me. Though, I wonder if 
>> `getTemplateArgumentBindingsText` should produce the leading space, that way 
>> we could just pass the result of it directly, rather than have to create a 
>> SmallString everywhere.  WDYT?
>
> There are other places (outside SemaOverload) that use this function and 
> don't need a space, though.
> I thought about just putting the space in the diagnostic itself, but then 
> sometimes you don't get template arguments and it fails.

I see.  I was hoping that wasn't the case!  OK, LGTM.  Let me know if you need 
me to commit this for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121646

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

[PATCH] D88905: [Clang] Allow "ext_vector_type" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 415400.
simoll added a comment.

Make the sema check for bool vector arithmetic specific to ext_vector_type 
bool. This assures that we do not interfere with scalar bool + int vector 
arithmetic sema (`vector.cpp` test was failing before).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88905

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/Sema/ext_vector_casts.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-size-conditional.cpp
  clang/test/SemaOpenCL/ext_vectors.cl

Index: clang/test/SemaOpenCL/ext_vectors.cl
===
--- clang/test/SemaOpenCL/ext_vectors.cl
+++ clang/test/SemaOpenCL/ext_vectors.cl
@@ -5,6 +5,7 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++2021
 
 typedef float float4 __attribute__((ext_vector_type(4)));
+typedef __attribute__((ext_vector_type(8))) bool BoolVector; // expected-error {{invalid vector element type 'bool'}}
 
 void test_ext_vector_accessors(float4 V) {
   V = V.wzyx;
Index: clang/test/SemaCXX/vector-size-conditional.cpp
===
--- clang/test/SemaCXX/vector-size-conditional.cpp
+++ clang/test/SemaCXX/vector-size-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((ext_vector_type(8)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
   all_dependent(four_ints, four_uints, two_uints); // expected-note {{in instantiation of}}
   all_dependent(four_ints, four_uints, four_uints);
 }
Index: clang/test/SemaCXX/vector-bool.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/vector-bool.cpp
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++17
+// Note that this test depends on the size of long-long to be different from
+// int, so it specifies a triple.
+
+using 

[clang] 7262eac - Revert rG9c542a5a4e1ba36c24e48185712779df52b7f7a6 "Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO"

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-03-15T13:01:35Z
New Revision: 7262eacd41997d7ca262d83367e28998662c1b21

URL: 
https://github.com/llvm/llvm-project/commit/7262eacd41997d7ca262d83367e28998662c1b21
DIFF: 
https://github.com/llvm/llvm-project/commit/7262eacd41997d7ca262d83367e28998662c1b21.diff

LOG: Revert rG9c542a5a4e1ba36c24e48185712779df52b7f7a6 "Lower 
`@llvm.global_dtors` using `__cxa_atexit` on MachO"

Mane of the build bots are complaining: Unknown command line argument 
'-lower-global-dtors'

Added: 
llvm/lib/Target/WebAssembly/WebAssemblyLowerGlobalDtors.cpp

Modified: 
clang/lib/CodeGen/BackendUtil.cpp
llvm/docs/Passes.rst
llvm/include/llvm/CodeGen/CommandFlags.h
llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
llvm/include/llvm/InitializePasses.h
llvm/include/llvm/LinkAllPasses.h
llvm/include/llvm/Target/TargetOptions.h
llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h
llvm/include/llvm/Transforms/Utils.h
llvm/lib/CodeGen/CodeGen.cpp
llvm/lib/CodeGen/CommandFlags.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/CodeGen/TargetPassConfig.cpp
llvm/lib/Passes/PassBuilder.cpp
llvm/lib/Passes/PassRegistry.def
llvm/lib/Target/WebAssembly/CMakeLists.txt
llvm/lib/Target/WebAssembly/WebAssembly.h
llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
llvm/lib/Transforms/Utils/CMakeLists.txt
llvm/test/CodeGen/ARM/ctors_dtors.ll
llvm/test/CodeGen/X86/2011-08-29-InitOrder.ll

Removed: 
llvm/include/llvm/Transforms/Utils/LowerGlobalDtors.h
llvm/lib/Transforms/Utils/LowerGlobalDtors.cpp
llvm/test/Transforms/LowerGlobalDestructors/lower-global-dtors.ll



diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 716a565ee7871..490f5b3de1ff3 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -546,8 +546,6 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
   Options.BinutilsVersion =
   llvm::TargetMachine::parseBinutilsVersion(CodeGenOpts.BinutilsVersion);
   Options.UseInitArray = CodeGenOpts.UseInitArray;
-  Options.LowerGlobalDtorsViaCxaAtExit =
-  CodeGenOpts.RegisterGlobalDtorsWithAtExit;
   Options.DisableIntegratedAS = CodeGenOpts.DisableIntegratedAS;
   Options.CompressDebugSections = CodeGenOpts.getCompressDebugSections();
   Options.RelaxELFRelocations = CodeGenOpts.RelaxELFRelocations;

diff  --git a/llvm/docs/Passes.rst b/llvm/docs/Passes.rst
index 7c0666992e8f5..92f06496b4ef9 100644
--- a/llvm/docs/Passes.rst
+++ b/llvm/docs/Passes.rst
@@ -876,14 +876,6 @@ This pass expects :ref:`LICM ` to be run 
before it to hoist
 invariant conditions out of the loop, to make the unswitching opportunity
 obvious.
 
-``-lower-global-dtors``: Lower global destructors
-
-
-This pass lowers global module destructors (``llvm.global_dtors``) by creating
-wrapper functions that are registered as global constructors in
-``llvm.global_ctors`` and which contain a call to ``__cxa_atexit`` to register
-their destructor functions.
-
 ``-loweratomic``: Lower atomic intrinsics to non-atomic form
 
 

diff  --git a/llvm/include/llvm/CodeGen/CommandFlags.h 
b/llvm/include/llvm/CodeGen/CommandFlags.h
index 4424db4aa2e41..aa91367f65b80 100644
--- a/llvm/include/llvm/CodeGen/CommandFlags.h
+++ b/llvm/include/llvm/CodeGen/CommandFlags.h
@@ -95,8 +95,6 @@ std::string getTrapFuncName();
 
 bool getUseCtors();
 
-bool getLowerGlobalDtorsViaCxaAtExit();
-
 bool getRelaxELFRelocations();
 
 bool getDataSections();

diff  --git a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h 
b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
index 26bda8d5d239d..2a35987507446 100644
--- a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
+++ b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
@@ -119,9 +119,6 @@ class TargetLoweringObjectFileMachO : public 
TargetLoweringObjectFile {
 
   void Initialize(MCContext &Ctx, const TargetMachine &TM) override;
 
-  MCSection *getStaticDtorSection(unsigned Priority,
-  const MCSymbol *KeySym) const override;
-
   /// Emit the module flags that specify the garbage collection information.
   void emitModuleMetadata(MCStreamer &Streamer, Module &M) const override;
 

diff  --git a/llvm/include/llvm/InitializePasses.h 
b/llvm/include/llvm/InitializePasses.h
index 82aafe2744184..3a98bacef81d0 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -274,7 +274,6 @@ void initializeLowerAtomicLegacyPassPass(PassRegistry&);
 void initializeLowerConstantIntrinsicsPass(PassRegistry&);
 void initializeLowerEmuTLSPass(PassRegistry&);
 void initializeLowerExpectIntrinsicPass(PassRegistry&);
-void i

[PATCH] D121327: Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.
Herald added a subscriber: asb.

@yln I've reverted your commit at rG7262eacd41997d7ca262d83367e28998662c1b21 
 to try 
and get the buildbots green again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121327

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

[PATCH] D88905: [Clang] Allow "ext_vector_type" applied to Booleans

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks! Sema bits now LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88905

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

[PATCH] D121637: [PowerPC] Fix EmitPPCBuiltinExpr to emit arguments once

[Quinn Pham via Phabricator via cfe-commits]

quinnp updated this revision to Diff 415412.
quinnp added a comment.

Minor formatting update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121637

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/PowerPC/builtins-ppc-fastmath.c
  clang/test/CodeGen/PowerPC/builtins-ppc-int128.c
  clang/test/CodeGen/PowerPC/builtins-ppc-vsx.c
  clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-cas.c
  clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-fetch.c
  clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-fp.c
  clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-rotate.c
  clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-sync.c

Index: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-sync.c
===
--- clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-sync.c
+++ clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-sync.c
@@ -14,13 +14,11 @@
 
 // CHECK-LABEL: @test_popcntb(
 // CHECK:[[TMP0:%.*]] = load i64, i64* @a, align 8
-// CHECK-NEXT:[[TMP1:%.*]] = load i64, i64* @a, align 8
 // CHECK-NEXT:[[POPCNTB:%.*]] = call i64 @llvm.ppc.popcntb.i64.i64(i64 [[TMP0]])
 // CHECK-NEXT:ret i64 [[POPCNTB]]
 //
 // CHECK-32-LABEL: @test_popcntb(
 // CHECK-32:[[TMP0:%.*]] = load i32, i32* @a, align 4
-// CHECK-32-NEXT:[[TMP1:%.*]] = load i32, i32* @a, align 4
 // CHECK-32-NEXT:[[POPCNTB:%.*]] = call i32 @llvm.ppc.popcntb.i32.i32(i32 [[TMP0]])
 // CHECK-32-NEXT:ret i32 [[POPCNTB]]
 //
@@ -198,13 +196,11 @@
 
 // CHECK-LABEL: @test_builtin_ppc_popcntb(
 // CHECK:[[TMP0:%.*]] = load i64, i64* @a, align 8
-// CHECK-NEXT:[[TMP1:%.*]] = load i64, i64* @a, align 8
 // CHECK-NEXT:[[POPCNTB:%.*]] = call i64 @llvm.ppc.popcntb.i64.i64(i64 [[TMP0]])
 // CHECK-NEXT:ret i64 [[POPCNTB]]
 //
 // CHECK-32-LABEL: @test_builtin_ppc_popcntb(
 // CHECK-32:[[TMP0:%.*]] = load i32, i32* @a, align 4
-// CHECK-32-NEXT:[[TMP1:%.*]] = load i32, i32* @a, align 4
 // CHECK-32-NEXT:[[POPCNTB:%.*]] = call i32 @llvm.ppc.popcntb.i32.i32(i32 [[TMP0]])
 // CHECK-32-NEXT:ret i32 [[POPCNTB]]
 //
Index: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-rotate.c
===
--- clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-rotate.c
+++ clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-rotate.c
@@ -15,11 +15,11 @@
   // CHECK-LABEL: test_builtin_ppc_rldimi
   // CHECK:   %res = alloca i64, align 8
   // CHECK-NEXT:  [[RA:%[0-9]+]] = load i64, i64* @ull, align 8
-  // CHECK-NEXT:  [[RB:%[0-9]+]] = load i64, i64* @ull, align 8
-  // CHECK-NEXT:  [[RC:%[0-9]+]] = call i64 @llvm.fshl.i64(i64 [[RA]], i64 [[RA]], i64 63)
-  // CHECK-NEXT:  [[RD:%[0-9]+]] = and i64 [[RC]], 72057593769492480
-  // CHECK-NEXT:  [[RE:%[0-9]+]] = and i64 [[RB]], -72057593769492481
-  // CHECK-NEXT:  [[RF:%[0-9]+]] = or i64 [[RD]], [[RE]]
+  // CHECK-NEXT:  [[RB:%[0-9]+]] = call i64 @llvm.fshl.i64(i64 [[RA]], i64 [[RA]], i64 63)
+  // CHECK-NEXT:  [[RC:%[0-9]+]] = and i64 [[RB]], 72057593769492480
+  // CHECK-NEXT:  [[RD:%[0-9]+]] = load i64, i64* @ull, align 8
+  // CHECK-NEXT:  [[RE:%[0-9]+]] = and i64 [[RD]], -72057593769492481
+  // CHECK-NEXT:  [[RF:%[0-9]+]] = or i64 [[RC]], [[RE]]
   // CHECK-NEXT:  store i64 [[RF]], i64* %res, align 8
   // CHECK-NEXT:  ret void
 
@@ -31,11 +31,11 @@
   // CHECK-LABEL: test_builtin_ppc_rlwimi
   // CHECK:   %res = alloca i32, align 4
   // CHECK-NEXT:  [[RA:%[0-9]+]] = load i32, i32* @ui, align 4
-  // CHECK-NEXT:  [[RB:%[0-9]+]] = load i32, i32* @ui, align 4
-  // CHECK-NEXT:  [[RC:%[0-9]+]] = call i32 @llvm.fshl.i32(i32 [[RA]], i32 [[RA]], i32 31)
-  // CHECK-NEXT:  [[RD:%[0-9]+]] = and i32 [[RC]], 16776960
-  // CHECK-NEXT:  [[RE:%[0-9]+]] = and i32 [[RB]], -16776961
-  // CHECK-NEXT:  [[RF:%[0-9]+]] = or i32 [[RD]], [[RE]]
+  // CHECK-NEXT:  [[RB:%[0-9]+]] = call i32 @llvm.fshl.i32(i32 [[RA]], i32 [[RA]], i32 31)
+  // CHECK-NEXT:  [[RC:%[0-9]+]] = and i32 [[RB]], 16776960
+  // CHECK-NEXT:  [[RD:%[0-9]+]] = load i32, i32* @ui, align 4
+  // CHECK-NEXT:  [[RE:%[0-9]+]] = and i32 [[RD]], -16776961
+  // CHECK-NEXT:  [[RF:%[0-9]+]] = or i32 [[RC]], [[RE]]
   // CHECK-NEXT:  store i32 [[RF]], i32* %res, align 4
   // CHECK-NEXT:  ret void
 
Index: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-fp.c
===
--- clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-fp.c
+++ clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-fp.c
@@ -15,9 +15,8 @@
 
 // CHECK-LABEL: @test_fric(
 // CHECK:[[TMP0:%.*]] = load double, double* @a, align 8
-// CHECK-NEXT:[[TMP1:%.*]] = load double, double* @a, align 8
-// CHECK-NEXT:[[TMP2:%.*]] = call double @llvm.rint.f64(double [[TMP1]])
-// CHECK-NEXT:ret double [[TMP2]]
+// CHECK-NEXT:[[TMP1:%.*]] = call double @llvm.rint.f64(double [[TMP0]])
+// CHECK-NEXT:ret double [[TMP

[PATCH] D121576: [clang-format] Don't unwrap lines preceded by line comments

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

In D121576#3381295 , @owenpan wrote:

> Right. I think we should have a specification (including when not to align 
> the cells) and re-design the entire thing. It's not maintainable as is. I 
> have some ideas but need to remove/change some of the existing test cases, 
> e.g., those involving concatenated string literals. If you (and @curdeius and 
> @HazardyKnusperkeks) agree, I can give it a shot.

It will be appreciated!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121576

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

[PATCH] D111587: re-land: [clang] Fix absolute file paths with -fdebug-prefix-map

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D111587#3381369 , @keith wrote:

> Fix tests with dwarf 6

Do you mean dwarf 5 here?  There is no v6 yet.




Comment at: clang/test/Modules/module-debuginfo-prefix.m:24
 
-// Dir should always be empty, but on Windows we can't recognize /var
-// as being an absolute path.
-// CHECK: !DIFile(filename: "/OVERRIDE/DebugObjC.h", directory: 
"{{()|(.*:.*)}}")
+// CHECK: !DIFile(filename: "{{/|.:}}OVERRIDE{{/|}}DebugObjC.h", 
directory: "")

Does this want to be 
`"%{fs-src-root}OVERRIDE%{fs-sep}DebugObjC.h"` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111587

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

[clang] 3524535 - [AARCH64] ssbs should be enabled by default for cortex-x1, cortex-x1c, cortex-a77

[Ties Stuij via cfe-commits]

Author: Ties Stuij
Date: 2022-03-15T13:44:20Z
New Revision: 352453569b2b044ddd5bd4df0074ff9863828b6f

URL: 
https://github.com/llvm/llvm-project/commit/352453569b2b044ddd5bd4df0074ff9863828b6f
DIFF: 
https://github.com/llvm/llvm-project/commit/352453569b2b044ddd5bd4df0074ff9863828b6f.diff

LOG: [AARCH64] ssbs should be enabled by default for cortex-x1, cortex-x1c, 
cortex-a77

Reviewed By: amilendra

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

Added: 


Modified: 
clang/test/Driver/aarch64-ssbs.c
clang/test/Preprocessor/aarch64-target-features.c
llvm/lib/Support/AArch64TargetParser.cpp
llvm/lib/Target/AArch64/AArch64.td

Removed: 




diff  --git a/clang/test/Driver/aarch64-ssbs.c 
b/clang/test/Driver/aarch64-ssbs.c
index 86c93ae926404..209255405d28d 100644
--- a/clang/test/Driver/aarch64-ssbs.c
+++ b/clang/test/Driver/aarch64-ssbs.c
@@ -1,7 +1,11 @@
 // RUN: %clang -### -target aarch64-none-none-eabi -march=armv8a+ssbs   %s 
2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -mcpu=cortex-x1  %s 
2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -mcpu=cortex-x1c %s 
2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -mcpu=cortex-a77 %s 
2>&1 | FileCheck %s
 // CHECK: "-target-feature" "+ssbs"
 
 // RUN: %clang -### -target aarch64-none-none-eabi -march=armv8a+nossbs %s 
2>&1 | FileCheck %s --check-prefix=NOSSBS
+// RUN: %clang -### -target aarch64-none-none-eabi -mcpu=cortex-x1c+nossbs %s 
2>&1 | FileCheck %s --check-prefix=NOSSBS
 // NOSSBS: "-target-feature" "-ssbs"
 
 // RUN: %clang -### -target aarch64-none-none-eabi  %s 
2>&1 | FileCheck %s --check-prefix=ABSENTSSBS

diff  --git a/clang/test/Preprocessor/aarch64-target-features.c 
b/clang/test/Preprocessor/aarch64-target-features.c
index 833d75b7e5b9e..b7e0113131ea7 100644
--- a/clang/test/Preprocessor/aarch64-target-features.c
+++ b/clang/test/Preprocessor/aarch64-target-features.c
@@ -285,7 +285,7 @@
 // CHECK-MCPU-A57: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" 
"-target-feature" "+crc" "-target-feature" "+crypto"
 // CHECK-MCPU-A72: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" 
"-target-feature" "+crc" "-target-feature" "+crypto"
 // CHECK-MCPU-CORTEX-A73: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" 
"-target-feature" "+v8a" "-target-feature" "+fp-armv8" "-target-feature" 
"+neon" "-target-feature" "+crc" "-target-feature" "+crypto"
-// CHECK-MCPU-CORTEX-R82: "-cc1"{{.*}} "-triple" "aarch64{{.*}}"  
"-target-feature" "+v8r" "-target-feature" "+fp-armv8" "-target-feature" 
"+neon" "-target-feature" "+crc" "-target-feature" "+dotprod" "-target-feature" 
"+fp16fml" "-target-feature" "+ras" "-target-feature" "+lse" "-target-feature" 
"+rdm" "-target-feature" "+rcpc" "-target-feature" "+fullfp16"
+// CHECK-MCPU-CORTEX-R82: "-cc1"{{.*}} "-triple" "aarch64{{.*}}"  
"-target-feature" "+v8r" "-target-feature" "+fp-armv8" "-target-feature" 
"+neon" "-target-feature" "+crc" "-target-feature" "+dotprod" "-target-feature" 
"+fp16fml" "-target-feature" "+ras" "-target-feature" "+lse" "-target-feature" 
"+rdm" "-target-feature" "+rcpc" "-target-feature" "+ssbs" "-target-feature" 
"+fullfp16"
 // CHECK-MCPU-M3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" 
"-target-feature" "+crc" "-target-feature" "+crypto"
 // CHECK-MCPU-M4: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"+neon" "-target-feature" "+crc" "-target-feature" "+crypto" "-target-feature" 
"+dotprod" "-target-feature" "+fullfp16"
 // CHECK-MCPU-KRYO: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" 
"-target-feature" "+crc" "-target-feature" "+crypto"

diff  --git a/llvm/lib/Support/AArch64TargetParser.cpp 
b/llvm/lib/Support/AArch64TargetParser.cpp
index cdf7c8ade9aac..bb19e2714be10 100644
--- a/llvm/lib/Support/AArch64TargetParser.cpp
+++ b/llvm/lib/Support/AArch64TargetParser.cpp
@@ -120,6 +120,8 @@ bool AArch64::getExtensionFeatures(uint64_t Extensions,
 Features.push_back("+mops");
   if (Extensions & AArch64::AEK_PERFMON)
 Features.push_back("+perfmon");
+  if (Extensions & AArch64::AEK_SSBS)
+Features.push_back("+ssbs");
 
   return true;
 }

diff  --git a/llvm/lib/Target/AArch64/AArch64.td 
b/llvm/lib/Target/AArch64/AArch64.td
index f53218c35d46e..872343e8b8f89 100644
--- a/llvm/lib/Target/AArch64/AArch64.td
+++ b/llvm/lib/Target/AArch64/AArch64.td
@@ -957,7 +957,7 @@ def ProcessorFeatures {
  FeatureRCPC, FeatureSSBS, FeaturePerfMon];
   list A77  = [HasV8_2aOps, FeatureCrypto, FeatureFPARMv8,
  FeatureNEO

[PATCH] D121206: [AARCH64] ssbs should be enabled by default for cortex-x1, cortex-x1c, cortex-a77

[Ties Stuij via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG352453569b2b: [AARCH64] ssbs should be enabled by default 
for cortex-x1, cortex-x1c, cortex… (authored by stuij).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121206

Files:
  clang/test/Driver/aarch64-ssbs.c
  clang/test/Preprocessor/aarch64-target-features.c
  llvm/lib/Support/AArch64TargetParser.cpp
  llvm/lib/Target/AArch64/AArch64.td


Index: llvm/lib/Target/AArch64/AArch64.td
===
--- llvm/lib/Target/AArch64/AArch64.td
+++ llvm/lib/Target/AArch64/AArch64.td
@@ -957,7 +957,7 @@
  FeatureRCPC, FeatureSSBS, FeaturePerfMon];
   list A77  = [HasV8_2aOps, FeatureCrypto, FeatureFPARMv8,
  FeatureNEON, FeatureFullFP16, FeatureDotProd,
- FeatureRCPC, FeaturePerfMon];
+ FeatureRCPC, FeaturePerfMon, FeatureSSBS];
   list A78  = [HasV8_2aOps, FeatureCrypto, FeatureFPARMv8,
  FeatureNEON, FeatureFullFP16, FeatureDotProd,
  FeatureRCPC, FeaturePerfMon, FeatureSPE,
@@ -975,11 +975,12 @@
  FeatureSB];
   list X1   = [HasV8_2aOps, FeatureCrypto, FeatureFPARMv8,
  FeatureNEON, FeatureRCPC, FeaturePerfMon,
- FeatureSPE, FeatureFullFP16, FeatureDotProd];
+ FeatureSPE, FeatureFullFP16, FeatureDotProd,
+ FeatureSSBS];
   list X1C  = [HasV8_2aOps, FeatureCrypto, FeatureFPARMv8,
  FeatureNEON, FeatureRCPC, FeaturePerfMon,
  FeatureSPE, FeatureFullFP16, FeatureDotProd,
- FeaturePAuth];
+ FeaturePAuth, FeatureSSBS];
   list X2   = [HasV9_0aOps, FeatureNEON, FeaturePerfMon,
  FeatureMatMulInt8, FeatureBF16, FeatureAM,
  FeatureMTE, FeatureETE, FeatureSVE2BitPerm,
Index: llvm/lib/Support/AArch64TargetParser.cpp
===
--- llvm/lib/Support/AArch64TargetParser.cpp
+++ llvm/lib/Support/AArch64TargetParser.cpp
@@ -120,6 +120,8 @@
 Features.push_back("+mops");
   if (Extensions & AArch64::AEK_PERFMON)
 Features.push_back("+perfmon");
+  if (Extensions & AArch64::AEK_SSBS)
+Features.push_back("+ssbs");
 
   return true;
 }
Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -285,7 +285,7 @@
 // CHECK-MCPU-A57: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" 
"-target-feature" "+crc" "-target-feature" "+crypto"
 // CHECK-MCPU-A72: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" 
"-target-feature" "+crc" "-target-feature" "+crypto"
 // CHECK-MCPU-CORTEX-A73: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" 
"-target-feature" "+v8a" "-target-feature" "+fp-armv8" "-target-feature" 
"+neon" "-target-feature" "+crc" "-target-feature" "+crypto"
-// CHECK-MCPU-CORTEX-R82: "-cc1"{{.*}} "-triple" "aarch64{{.*}}"  
"-target-feature" "+v8r" "-target-feature" "+fp-armv8" "-target-feature" 
"+neon" "-target-feature" "+crc" "-target-feature" "+dotprod" "-target-feature" 
"+fp16fml" "-target-feature" "+ras" "-target-feature" "+lse" "-target-feature" 
"+rdm" "-target-feature" "+rcpc" "-target-feature" "+fullfp16"
+// CHECK-MCPU-CORTEX-R82: "-cc1"{{.*}} "-triple" "aarch64{{.*}}"  
"-target-feature" "+v8r" "-target-feature" "+fp-armv8" "-target-feature" 
"+neon" "-target-feature" "+crc" "-target-feature" "+dotprod" "-target-feature" 
"+fp16fml" "-target-feature" "+ras" "-target-feature" "+lse" "-target-feature" 
"+rdm" "-target-feature" "+rcpc" "-target-feature" "+ssbs" "-target-feature" 
"+fullfp16"
 // CHECK-MCPU-M3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" 
"-target-feature" "+crc" "-target-feature" "+crypto"
 // CHECK-MCPU-M4: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"+neon" "-target-feature" "+crc" "-target-feature" "+crypto" "-target-feature" 
"+dotprod" "-target-feature" "+fullfp16"
 // CHECK-MCPU-KRYO: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" 
"-target-feature" "+crc" "-target-feature" "+crypto"
Index: clang/test/Driver/aarch64-ssbs.c
==

[PATCH] D119479: [clang][extract-api] Add global record support

[Daniel Grumberg via Phabricator via cfe-commits]

dang requested changes to this revision.
dang added a comment.
This revision now requires changes to proceed.

You should fix the test to take into account the serializer feedback I left 
behind




Comment at: clang/include/clang/SymbolGraph/API.h:33
+
+struct APIRecord {
+  StringRef Name;

Might be worth deleting the default constructor.



Comment at: clang/include/clang/SymbolGraph/API.h:44
+  /// Discriminator for LLVM-style RTTI (dyn_cast<> et al.)
+  enum RecordKind {
+RK_Global,

APIRecord isn't an abstract class so can be instantiated. To be strictly 
compliant with the LLVM-style RTTI you would need an enum case for a plain 
record. Otherwise you could introduce a pure virtual function to APIRecord to 
make abstract. A good candidate for that would be the destructor, if you make 
that pure virtual but provide an empty implementation, the behaviour should be 
what you want.



Comment at: clang/lib/AST/RawCommentList.cpp:366
 
+  auto DropTrailingNewLines = [](std::string &Str) {
+while (!Str.empty() && Str.back() == '\n')

Consider not using a lambda for this since you only do this once. Also consider 
rewriting the logic as
```
auto LastChar = S.find_last_not_of("\n");
S.erase(LastChar+1, S.size())
```



Comment at: clang/lib/AST/RawCommentList.cpp:431
 }
+std::string Line;
 llvm::StringRef TokText = L.getSpelling(Tok, SourceMgr);

Might want to use a `SmallString<124>` here as it is likely that the comment 
line would fit into that and the `+=` operator would only operate on stack 
storage. In the end of the common case (the line fits in 124 chars) you would 
only get a single heap allocation when you store emplace it into `Results`.



Comment at: clang/lib/SymbolGraph/DeclarationFragments.cpp:54
+  case DeclarationFragments::FragmentKind::GenericParameter:
+return "genericParam";
+  case DeclarationFragments::FragmentKind::ExternalParam:

"genericParameter" is what is used in SymbolKit (c.f. 
https://github.com/apple/swift-docc-symbolkit/blob/main/Sources/SymbolKit/SymbolGraph/Symbol/DeclarationFragments/Fragment/FragmentKind.swift)



Comment at: clang/lib/SymbolGraph/DeclarationFragments.cpp:76
+DeclarationFragments::FragmentKind::TypeIdentifier)
+  .Case("genericParam",
+DeclarationFragments::FragmentKind::GenericParameter)

ditto "genericParameter"



Comment at: clang/lib/SymbolGraph/ExtractAPIConsumer.cpp:173
+public:
+  explicit ExtractAPIConsumer(ASTContext &Context,
+  std::unique_ptr OS)

`explicit` doesn't do anything here since this constructor takes in 2 arguments?



Comment at: clang/lib/SymbolGraph/Serialization.cpp:123
+  case Language::ObjC:
+return "objc";
+

Should be "occ"



Comment at: clang/lib/SymbolGraph/Serialization.cpp:233
+case GVKind::Function:
+  Kind["identifier"] = (getLanguageName(LangOpts) + ".function").str();
+  Kind["displayName"] = "Function";

".func"



Comment at: clang/lib/SymbolGraph/Serialization.cpp:237
+case GVKind::Variable:
+  Kind["identifier"] = (getLanguageName(LangOpts) + ".variable").str();
+  Kind["displayName"] = "Variable";

".variable"



Comment at: clang/lib/SymbolGraph/Serialization.cpp:238
+  Kind["identifier"] = (getLanguageName(LangOpts) + ".variable").str();
+  Kind["displayName"] = "Variable";
+  break;

"Global Variable"



Comment at: clang/lib/SymbolGraph/Serialization.cpp:252
+
+const VersionTuple Serializer::FormatVersion{0, 5, 3};
+

Currently the spec 
(https://github.com/apple/swift-docc-symbolkit/blob/main/openapi.yaml) lists 
0.3.0 as the version. Let's use that.



Comment at: clang/lib/SymbolGraph/Serialization.cpp:258
+  serializeSemanticVersion(FormatVersion));
+  Metadata["generator"] = "clang";
+  return Metadata;

something more descriptive would be nice here. I think you want the output off 
`getClangFullRepositoryVersion` or `getClangFullVersion` so we have something 
meaningful to go off for future bugs.



Comment at: clang/lib/SymbolGraph/Serialization.cpp:264
+  Object Module;
+  // FIXME: What to put in here?
+  Module["name"] = "";

As discussed offline we need to pass clang an extra flag to populate this. I 
suggest `--product-name`.



Comment at: clang/lib/SymbolGraph/Serialization.cpp:283
+  serializeObject(Obj, "docComment", serializeDocComment(Record.Comment));
+  serializeArray(Obj, "declaration",
+ serializeDeclarationFragments(Record.Declaration));

This is called "declar

[PATCH] D121694: [clang][dataflow] Allow disabling built-in transfer functions for CFG terminators

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: sgatev, xazax.hun.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

Terminators are handled specially in the transfer functions so we need an
additional check on whether the analysis has disabled built-in transfer
functions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121694

Files:
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -170,6 +170,8 @@
   }
 
   llvm::Optional MaybeState;
+  bool ApplyBuiltinTransfer = Analysis.applyBuiltinTransfer();
+
   for (const CFGBlock *Pred : Preds) {
 // Skip if the `Block` is unreachable or control flow cannot get past it.
 if (!Pred || Pred->hasNoReturnElement())
@@ -183,11 +185,13 @@
   continue;
 
 TypeErasedDataflowAnalysisState PredState = MaybePredState.getValue();
-if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
-  const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates);
-  TerminatorVisitor(StmtToEnv, PredState.Env,
-blockIndexInPredecessor(*Pred, Block))
-  .Visit(PredTerminatorStmt);
+if (ApplyBuiltinTransfer) {
+  if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
+const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates);
+TerminatorVisitor(StmtToEnv, PredState.Env,
+  blockIndexInPredecessor(*Pred, Block))
+.Visit(PredTerminatorStmt);
+  }
 }
 
 if (MaybeState.hasValue()) {


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -170,6 +170,8 @@
   }
 
   llvm::Optional MaybeState;
+  bool ApplyBuiltinTransfer = Analysis.applyBuiltinTransfer();
+
   for (const CFGBlock *Pred : Preds) {
 // Skip if the `Block` is unreachable or control flow cannot get past it.
 if (!Pred || Pred->hasNoReturnElement())
@@ -183,11 +185,13 @@
   continue;
 
 TypeErasedDataflowAnalysisState PredState = MaybePredState.getValue();
-if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
-  const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates);
-  TerminatorVisitor(StmtToEnv, PredState.Env,
-blockIndexInPredecessor(*Pred, Block))
-  .Visit(PredTerminatorStmt);
+if (ApplyBuiltinTransfer) {
+  if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
+const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates);
+TerminatorVisitor(StmtToEnv, PredState.Env,
+  blockIndexInPredecessor(*Pred, Block))
+.Visit(PredTerminatorStmt);
+  }
 }
 
 if (MaybeState.hasValue()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111400: [Clang] Implement P2242R3

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:5128-5130
+  if (!CheckLocalVariableDeclaration(Info, VD)) {
+return ESR_Failed;
+  }





Comment at: clang/lib/AST/ExprConstant.cpp:5174-5177
+  const VarDecl *VD = dyn_cast_or_null(D);
+  if (VD && !CheckLocalVariableDeclaration(Info, VD)) {
+return ESR_Failed;
+  }





Comment at: clang/lib/Sema/SemaDeclCXX.cpp:2130
+  case Stmt::GotoStmtClass:
+if (!Cxx2bLoc.isValid())
+  Cxx2bLoc = S->getBeginLoc();





Comment at: clang/lib/Sema/SemaDeclCXX.cpp:2132-2136
+for (Stmt *SubStmt : S->children())
+  if (SubStmt &&
+  !CheckConstexprFunctionStmt(SemaRef, Dcl, SubStmt, ReturnStmts,
+  Cxx1yLoc, Cxx2aLoc, Cxx2bLoc, Kind))
 return false;





Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp:1
-// RUN: %clang_cc1 -std=c++2a -verify %s
 

I think we want to keep testing the C++20 behavior and want new tests for the 
C++2b behavior. We still need to be sure we're correct in C++20 mode given the 
differences (even in extensions because `-pedantic-errors` is a thing). So I'd 
recommend splitting this test into two or using an additional RUN line.



Comment at: clang/test/SemaCXX/constant-expression-cxx14.cpp:47
 static_assert(g(2) == 42, "");
-constexpr int h(int n) {
-  static const int m = n; // expected-error {{static variable not permitted in 
a constexpr function}}

I think we should keep this test coverage by adding `-Werror=c++2b-extensions` 
to the RUN line for C++2b.



Comment at: clang/test/SemaCXX/constant-expression-cxx14.cpp:61
+} // expected-warning {{non-void}} \
+  //expected-note 2{{control reached end of constexpr function}}
+





Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:8-9
+}
+constexpr int g(int n) {//  expected-error {{constexpr function never 
produces a constant expression}}
+  thread_local const int m = n; //  expected-warning {{definition of a 
thread_local variable in a constexpr function is incompatible with C++ 
standards before C++2b}} \
+// expected-note {{control flows through the 
declaration of a thread_local variable}}





Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:60-61
+
+constexpr int ke = k_evaluated(1); //expected-error {{constexpr variable 'ke' 
must be initialized by a constant expression}} \
+   //expected-note {{in call}}
+

This error seems suspect to me. If we made flowing through a thread_local into 
an extension (line 54), then this code should be accepted. However, I think 
we're getting the error because the constant expression evaluator produces the 
note on line 55 and that usually will cause the evaluator to claim it wasn't a 
constant expression.



Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:77
+int a = f(0);
+constexpr int b = f(0); //expected-error {{must be initialized by a constant 
expression}} \
+// expected-note {{in call to 'f(0)'}}




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D111400: [Clang] Implement P2242R3

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 415425.
cor3ntin marked 6 inline comments as done.
cor3ntin added a comment.

Address formatting and style issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaCXX/constant-expression-cxx2b.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1361,7 +1361,7 @@
 
   Non-literal variables (and labels and gotos) in constexpr functions
   https://wg21.link/P2242R3";>P2242R3
-  No
+  Clang 15
 
 
   Character encoding of diagnostic text
Index: clang/test/SemaCXX/constant-expression-cxx2b.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constant-expression-cxx2b.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify %s -fcxx-exceptions -triple=x86_64-linux-gnu -Wpre-c++2b-compat
+
+constexpr int f(int n) {  // expected-error {{constexpr function never produces a constant expression}}
+  static const int m = n; // expected-warning {{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+  // expected-note {{control flows through the declaration of a static variable}}
+  return m;
+}
+constexpr int g(int n) {// expected-error {{constexpr function never produces a constant expression}}
+  thread_local const int m = n; // expected-warning {{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+// expected-note {{control flows through the declaration of a thread_local variable}}
+  return m;
+}
+
+constexpr int h(int n) {  // expected-error {{constexpr function never produces a constant expression}}
+  static const int m = n; // expected-warning {{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+  // expected-note {{control flows through the declaration of a static variable}}
+  return &m - &m;
+}
+constexpr int i(int n) {// expected-error {{constexpr function never produces a constant expression}}
+  thread_local const int m = n; // expected-warning {{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+// expected-note {{control flows through the declaration of a thread_local variable}}
+  return &m - &m;
+}
+
+constexpr int j(int n) {
+  if (!n)
+return 0;
+  static const int m = n; // expected-warning{{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}}
+  return m;
+}
+
+constexpr int k(int n) {
+  if (!n)
+return 0;
+  thread_local const int m = n; // expected-warning{{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+
+  return m;
+}
+
+constexpr int j_evaluated(int n) {
+  if (!n)
+return 0;
+  static const int m = n; // expected-warning{{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+  // expected-note {{control flows through the declaration of a static variable}}
+  return m;
+}
+
+constexpr int je = j_evaluated(1); // expected-error {{constexpr variable 'je' must be initialized by a constant expression}}  \
+   // expected-note {{in call}}
+
+constexpr int k_evaluated(int n) {
+  if (!n)
+return 0;
+  thread_local const int m = n; // expected-warning{{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+// expected-note {{control flows through the declaration of a thread_local variable}}
+
+  return m;
+}
+
+constexpr int ke = k_evaluated(1); // expected-error {{constexpr variable 'ke' must be initialized by a constant expression}} \
+   // expected-note {{in call}}
+
+namespace eval_goto {
+
+constexpr int f(int x) {
+  if (x) {
+return 0;
+  } else {
+goto test; // expected-note {{subexpression not valid in a constant expression}} \
+// expected-warning {{us

[PATCH] D111400: [Clang] Implement P2242R3

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:2132-2136
+for (Stmt *SubStmt : S->children())
+  if (SubStmt &&
+  !CheckConstexprFunctionStmt(SemaRef, Dcl, SubStmt, ReturnStmts,
+  Cxx1yLoc, Cxx2aLoc, Cxx2bLoc, Kind))
 return false;

aaron.ballman wrote:
> 
This is consistent with the same code above. Should I be inconsistent?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D119479: [clang][extract-api] Add global record support

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/lib/SymbolGraph/Serialization.cpp:237
+case GVKind::Variable:
+  Kind["identifier"] = (getLanguageName(LangOpts) + ".variable").str();
+  Kind["displayName"] = "Variable";

dang wrote:
> ".variable"
".var" sorry


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119479

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

[PATCH] D111400: [Clang] Implement P2242R3

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:2132-2136
+for (Stmt *SubStmt : S->children())
+  if (SubStmt &&
+  !CheckConstexprFunctionStmt(SemaRef, Dcl, SubStmt, ReturnStmts,
+  Cxx1yLoc, Cxx2aLoc, Cxx2bLoc, Kind))
 return false;

cor3ntin wrote:
> aaron.ballman wrote:
> > 
> This is consistent with the same code above. Should I be inconsistent?
I don't have strong opinions, either way is defensible. I just have a hard time 
reading the code with nesting the substatements like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D111400: [Clang] Implement P2242R3

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:60-61
+
+constexpr int ke = k_evaluated(1); //expected-error {{constexpr variable 'ke' 
must be initialized by a constant expression}} \
+   //expected-note {{in call}}
+

aaron.ballman wrote:
> This error seems suspect to me. If we made flowing through a thread_local 
> into an extension (line 54), then this code should be accepted. However, I 
> think we're getting the error because the constant expression evaluator 
> produces the note on line 55 and that usually will cause the evaluator to 
> claim it wasn't a constant expression.
We can not evaluate at compile time a thread_local or static, even if we allow 
them to appear in a `constexpr` function. So this is the behavior I'd expect


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D111400: [Clang] Implement P2242R3

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:2132-2136
+for (Stmt *SubStmt : S->children())
+  if (SubStmt &&
+  !CheckConstexprFunctionStmt(SemaRef, Dcl, SubStmt, ReturnStmts,
+  Cxx1yLoc, Cxx2aLoc, Cxx2bLoc, Kind))
 return false;

aaron.ballman wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > 
> > This is consistent with the same code above. Should I be inconsistent?
> I don't have strong opinions, either way is defensible. I just have a hard 
> time reading the code with nesting the substatements like this.
Would you be happy if i fix all of them later as a nfc?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D111400: [Clang] Implement P2242R3

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:60-61
+
+constexpr int ke = k_evaluated(1); //expected-error {{constexpr variable 'ke' 
must be initialized by a constant expression}} \
+   //expected-note {{in call}}
+

cor3ntin wrote:
> aaron.ballman wrote:
> > This error seems suspect to me. If we made flowing through a thread_local 
> > into an extension (line 54), then this code should be accepted. However, I 
> > think we're getting the error because the constant expression evaluator 
> > produces the note on line 55 and that usually will cause the evaluator to 
> > claim it wasn't a constant expression.
> We can not evaluate at compile time a thread_local or static, even if we 
> allow them to appear in a `constexpr` function. So this is the behavior I'd 
> expect
Yes, you're right, I lost that context before. Sorry for the noise, I think the 
error here is correct. But I also worry that users may get confused in the same 
way I just did. That said, I can't think of a better diagnostic wording and I 
think the behavior in situ will be fine because the note will be "attached" to 
the error in the diagnostic output, which makes the error more clear than 
reading the code statically in the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D111400: [Clang] Implement P2242R3

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:2132-2136
+for (Stmt *SubStmt : S->children())
+  if (SubStmt &&
+  !CheckConstexprFunctionStmt(SemaRef, Dcl, SubStmt, ReturnStmts,
+  Cxx1yLoc, Cxx2aLoc, Cxx2bLoc, Kind))
 return false;

cor3ntin wrote:
> aaron.ballman wrote:
> > cor3ntin wrote:
> > > aaron.ballman wrote:
> > > > 
> > > This is consistent with the same code above. Should I be inconsistent?
> > I don't have strong opinions, either way is defensible. I just have a hard 
> > time reading the code with nesting the substatements like this.
> Would you be happy if i fix all of them later as a nfc?
Absolutely! Or if you want to land an NFC fix now and rebase your patch on top 
of that, that'd also be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D121694: [clang][dataflow] Allow disabling built-in transfer functions for CFG terminators

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

xazax.hun accepted this revision.
xazax.hun added a comment.

The change itself looks good. But out of curiosity, could you give me an 
example when we do not want to use the builtin transfer functions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121694

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

[PATCH] D121694: [clang][dataflow] Allow disabling built-in transfer functions for CFG terminators

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D121694#3382473 , @xazax.hun wrote:

> The change itself looks good. But out of curiosity, could you give me an 
> example when we do not want to use the builtin transfer functions?

Sure! Pretty much any plain-vanilla dataflow analysis that sticks to its own 
lattice and doesn't care about the environment. The demo constant-propagation 
analyses are like this, but we have additional real analyses using the 
framework in this way. Examples include an analysis to detect raw pointers that 
could be unique pointers and one that detects missed opportunies to use 
`std::move`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121694

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

[PATCH] D111400: [Clang] Implement P2242R3

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 415438.
cor3ntin added a comment.

Restore the C++14 tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaCXX/constant-expression-cxx2b.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1361,7 +1361,7 @@
 
   Non-literal variables (and labels and gotos) in constexpr functions
   https://wg21.link/P2242R3";>P2242R3
-  No
+  Clang 15
 
 
   Character encoding of diagnostic text
Index: clang/test/SemaCXX/constant-expression-cxx2b.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constant-expression-cxx2b.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify %s -fcxx-exceptions -triple=x86_64-linux-gnu -Wpre-c++2b-compat
+
+constexpr int f(int n) {  // expected-error {{constexpr function never produces a constant expression}}
+  static const int m = n; // expected-warning {{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+  // expected-note {{control flows through the declaration of a static variable}}
+  return m;
+}
+constexpr int g(int n) {// expected-error {{constexpr function never produces a constant expression}}
+  thread_local const int m = n; // expected-warning {{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+// expected-note {{control flows through the declaration of a thread_local variable}}
+  return m;
+}
+
+constexpr int h(int n) {  // expected-error {{constexpr function never produces a constant expression}}
+  static const int m = n; // expected-warning {{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+  // expected-note {{control flows through the declaration of a static variable}}
+  return &m - &m;
+}
+constexpr int i(int n) {// expected-error {{constexpr function never produces a constant expression}}
+  thread_local const int m = n; // expected-warning {{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+// expected-note {{control flows through the declaration of a thread_local variable}}
+  return &m - &m;
+}
+
+constexpr int j(int n) {
+  if (!n)
+return 0;
+  static const int m = n; // expected-warning{{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}}
+  return m;
+}
+
+constexpr int k(int n) {
+  if (!n)
+return 0;
+  thread_local const int m = n; // expected-warning{{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+
+  return m;
+}
+
+constexpr int j_evaluated(int n) {
+  if (!n)
+return 0;
+  static const int m = n; // expected-warning{{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+  // expected-note {{control flows through the declaration of a static variable}}
+  return m;
+}
+
+constexpr int je = j_evaluated(1); // expected-error {{constexpr variable 'je' must be initialized by a constant expression}}  \
+   // expected-note {{in call}}
+
+constexpr int k_evaluated(int n) {
+  if (!n)
+return 0;
+  thread_local const int m = n; // expected-warning{{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+// expected-note {{control flows through the declaration of a thread_local variable}}
+
+  return m;
+}
+
+constexpr int ke = k_evaluated(1); // expected-error {{constexpr variable 'ke' must be initialized by a constant expression}} \
+   // expected-note {{in call}}
+
+namespace eval_goto {
+
+constexpr int f(int x) {
+  if (x) {
+return 0;
+  } else {
+goto test; // expected-note {{subexpression not valid in a constant expression}} \
+// expected-warning {{use of this statement in a constexpr function is incompat

[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 415439.
jhuber6 added a comment.

We shouldn't need to restrict this to RDC only if implemented properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cuda-openmp-driver.cu

Index: clang/test/Driver/cuda-openmp-driver.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-openmp-driver.cu
@@ -0,0 +1,16 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings -fgpu-rdc \
+// RUN:-foffload-new-driver --offload-arch=sm_35 --offload-arch=sm_70 %s 2>&1 \
+// RUN: | FileCheck -check-prefix CHECK %s
+
+// CHECK: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[PTX_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[PTX_SM_35]]"], output: "[[CUBIN_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Linker", inputs: ["[[CUBIN_SM_35]]", "[[PTX_SM_35]]"], output: "[[FATBIN_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]"], output: "[[PTX_SM_70:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[PTX_SM_70:.+]]"], output: "[[CUBIN_SM_70:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Linker", inputs: ["[[CUBIN_SM_70]]", "[[PTX_SM_70:.+]]"], output: "[[FATBIN_SM_70:.+]]"
+// CHECK: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT]]", "[[FATBIN_SM_35]]", "[[FATBIN_SM_70]]"], output: "[[HOST_OBJ:.+]]"
+// CHECK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -71,8 +71,8 @@
   if (Args.hasArg(options::OPT_static))
 if (const Arg *A =
 Args.getLastArg(options::OPT_dynamic, options::OPT_mdynamic_no_pic))
-  D.Diag(diag::err_drv_argument_not_allowed_with) << A->getAsString(Args)
-  << "-static";
+  D.Diag(diag::err_drv_argument_not_allowed_with)
+  << A->getAsString(Args) << "-static";
 }
 
 // Add backslashes to escape spaces and other backslashes.
@@ -157,8 +157,8 @@
 /// parameter in reciprocal argument strings. Return false if there is an error
 /// parsing the refinement step. Otherwise, return true and set the Position
 /// of the refinement step in the input string.
-static bool getRefinementStep(StringRef In, const Driver &D,
-  const Arg &A, size_t &Position) {
+static bool getRefinementStep(StringRef In, const Driver &D, const Arg &A,
+  size_t &Position) {
   const char RefinementStepToken = ':';
   Position = In.find(RefinementStepToken);
   if (Position != StringRef::npos) {
@@ -510,7 +510,7 @@
 }
 
 static bool mustUseNonLeafFramePointerForTarget(const llvm::Triple &Triple) {
-  switch (Triple.getArch()){
+  switch (Triple.getArch()) {
   default:
 return false;
   case llvm::Triple::arm:
@@ -705,7 +705,7 @@
 
 /// Add a CC1 and CC1AS option to specify the coverage file path prefix map.
 static void addCoveragePrefixMapArg(const Driver &D, const ArgList &Args,
-   ArgStringList &CmdArgs) {
+ArgStringList &CmdArgs) {
   for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ,
 options::OPT_fcoverage_prefix_map_EQ)) {
 StringRef Map = A->getValue();
@@ -801,13 +801,12 @@
   CSPGOGenerateArg->getOption().matches(options::OPT_fno_profile_generate))
 CSPGOGenerateArg = nullptr;
 
-  auto *ProfileGenerateArg = Args.getLastArg(
-  options::OPT_fprofile_instr_generate,
-  options::OPT_fprofile_instr_generate_EQ,
-  options::OPT_fno_profile_instr_generate);
-  if (ProfileGenerateArg &&
-  ProfileGenerateArg->getOption().matches(
-  options::OPT_fno_profile_instr_generate))
+  auto *ProfileGenerateArg =
+  Args.getLastArg(options::OPT_fprofile_instr_generate,
+  options::OPT_fprofile_instr_generate_EQ,
+  options::OPT_fno_profile_instr_generate);
+  if (ProfileGenerateArg && ProfileGenerateArg->getOption().matches(
+options::OPT_fno_profile_instr_generate))
 ProfileGenerateArg = nullptr;
 
   if (PGOGenerateArg && ProfileGenerateArg)
@@ -1334,8 +1333,8 @@
   }
 
   if (ThroughHeader.empty()) {
-CmdArgs.push_back(Args.MakeArgString(
-Twine("-pch-through-hdrstop-") + (YcArg ? "create" : "use")));

[clang] 4633c02 - [clang][dataflow] Allow disabling built-in transfer functions for CFG terminators

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2022-03-15T15:10:32Z
New Revision: 4633c02eb0013ed38319b3fd708f59251bcd2aaf

URL: 
https://github.com/llvm/llvm-project/commit/4633c02eb0013ed38319b3fd708f59251bcd2aaf
DIFF: 
https://github.com/llvm/llvm-project/commit/4633c02eb0013ed38319b3fd708f59251bcd2aaf.diff

LOG: [clang][dataflow] Allow disabling built-in transfer functions for CFG 
terminators

Terminators are handled specially in the transfer functions so we need an
additional check on whether the analysis has disabled built-in transfer
functions.

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp 
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 058922afd4838..6baf1a7fd42d6 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -170,6 +170,8 @@ static TypeErasedDataflowAnalysisState 
computeBlockInputState(
   }
 
   llvm::Optional MaybeState;
+  bool ApplyBuiltinTransfer = Analysis.applyBuiltinTransfer();
+
   for (const CFGBlock *Pred : Preds) {
 // Skip if the `Block` is unreachable or control flow cannot get past it.
 if (!Pred || Pred->hasNoReturnElement())
@@ -183,11 +185,13 @@ static TypeErasedDataflowAnalysisState 
computeBlockInputState(
   continue;
 
 TypeErasedDataflowAnalysisState PredState = MaybePredState.getValue();
-if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
-  const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates);
-  TerminatorVisitor(StmtToEnv, PredState.Env,
-blockIndexInPredecessor(*Pred, Block))
-  .Visit(PredTerminatorStmt);
+if (ApplyBuiltinTransfer) {
+  if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
+const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates);
+TerminatorVisitor(StmtToEnv, PredState.Env,
+  blockIndexInPredecessor(*Pred, Block))
+.Visit(PredTerminatorStmt);
+  }
 }
 
 if (MaybeState.hasValue()) {



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

[PATCH] D121694: [clang][dataflow] Allow disabling built-in transfer functions for CFG terminators

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4633c02eb001: [clang][dataflow] Allow disabling built-in 
transfer functions for CFG… (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121694

Files:
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -170,6 +170,8 @@
   }
 
   llvm::Optional MaybeState;
+  bool ApplyBuiltinTransfer = Analysis.applyBuiltinTransfer();
+
   for (const CFGBlock *Pred : Preds) {
 // Skip if the `Block` is unreachable or control flow cannot get past it.
 if (!Pred || Pred->hasNoReturnElement())
@@ -183,11 +185,13 @@
   continue;
 
 TypeErasedDataflowAnalysisState PredState = MaybePredState.getValue();
-if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
-  const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates);
-  TerminatorVisitor(StmtToEnv, PredState.Env,
-blockIndexInPredecessor(*Pred, Block))
-  .Visit(PredTerminatorStmt);
+if (ApplyBuiltinTransfer) {
+  if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
+const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates);
+TerminatorVisitor(StmtToEnv, PredState.Env,
+  blockIndexInPredecessor(*Pred, Block))
+.Visit(PredTerminatorStmt);
+  }
 }
 
 if (MaybeState.hasValue()) {


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -170,6 +170,8 @@
   }
 
   llvm::Optional MaybeState;
+  bool ApplyBuiltinTransfer = Analysis.applyBuiltinTransfer();
+
   for (const CFGBlock *Pred : Preds) {
 // Skip if the `Block` is unreachable or control flow cannot get past it.
 if (!Pred || Pred->hasNoReturnElement())
@@ -183,11 +185,13 @@
   continue;
 
 TypeErasedDataflowAnalysisState PredState = MaybePredState.getValue();
-if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
-  const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates);
-  TerminatorVisitor(StmtToEnv, PredState.Env,
-blockIndexInPredecessor(*Pred, Block))
-  .Visit(PredTerminatorStmt);
+if (ApplyBuiltinTransfer) {
+  if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
+const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates);
+TerminatorVisitor(StmtToEnv, PredState.Env,
+  blockIndexInPredecessor(*Pred, Block))
+.Visit(PredTerminatorStmt);
+  }
 }
 
 if (MaybeState.hasValue()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121707: [llvm][AArch64] Insert "bti j" after call to setjmp

[David Spickett via Phabricator via cfe-commits]

DavidSpickett created this revision.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Some implementations of setjmp will end with a br instead of a ret.
This means that the next instruction after a call to setjmp must be
a "bti j" (j for jump) to make this work when branch target identification
is enabled.

The BTI extension was added in armv8.5-a but the bti instruction is in the
hint space. This means we can emit it for any architecture version as long
as branch target enforcement flags are passed.

The starting point for the hint number is 32 then call adds 2, jump adds 4.
Hence "hint #36" for a "bti j" (and "hint #34" for the "bti c" you see
at the start of functions).

The existing Arm command line option -mno-bti-at-return-twice has been
applied to AArch64 as well.

Support is added to SelectionDAG Isel and GlobalIsel. FastIsel will
defer to SelectionDAG.

Based on the change done for M profile Arm in https://reviews.llvm.org/D112427

This is for https://github.com/llvm/llvm-project/issues/4


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121707

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/setjmp-bti-no-enforcement.ll
  llvm/test/CodeGen/AArch64/setjmp-bti-outliner.ll
  llvm/test/CodeGen/AArch64/setjmp-bti.ll

Index: llvm/test/CodeGen/AArch64/setjmp-bti.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/setjmp-bti.ll
@@ -0,0 +1,55 @@
+; RUN: llc -mtriple=aarch64-none-linux-gnu < %s | FileCheck %s --check-prefix=BTI
+; RUN: llc -mtriple=aarch64-none-linux-gnu -global-isel < %s | FileCheck %s --check-prefix=BTI
+; RUN: llc -mtriple=aarch64-none-linux-gnu -fast-isel < %s | FileCheck %s --check-prefix=BTI
+; RUN: llc -mtriple=aarch64-none-linux-gnu -mattr=+no-bti-at-return-twice < %s | \
+; RUN: FileCheck %s --check-prefix=NOBTI
+; RUN: llc -mtriple=aarch64-none-linux-gnu -global-isel -mattr=+no-bti-at-return-twice < %s | \
+; RUN: FileCheck %s --check-prefix=NOBTI
+; RUN: llc -mtriple=aarch64-none-linux-gnu -fast-isel -mattr=+no-bti-at-return-twice < %s | \
+; RUN: FileCheck %s --check-prefix=NOBTI
+
+; C source
+; 
+; extern int setjmp(void*);
+; extern void notsetjmp(void);
+;
+; void bbb(void) {
+;   setjmp(0);
+;   int (*fnptr)(void*) = setjmp;
+;   fnptr(0);
+;   notsetjmp();
+; }
+
+define void @bbb() {
+; BTI-LABEL: bbb:
+; BTI:   bl setjmp
+; BTI-NEXT:  hint #36
+; BTI:   blr x{{[0-9]+}}
+; BTI-NEXT:  hint #36
+; BTI:   bl notsetjmp
+; BTI-NOT:   hint #36
+
+; NOBTI-LABEL: bbb:
+; NOBTI: bl setjmp
+; NOBTI-NOT: hint #36
+; NOBTI: blr x{{[0-9]+}}
+; NOBTI-NOT: hint #36
+; NOBTI: bl notsetjmp
+; NOBTI-NOT: hint #36
+entry:
+  %fnptr = alloca i32 (i8*)*, align 8
+  %call = call i32 @setjmp(i8* noundef null) #0
+  store i32 (i8*)* @setjmp, i32 (i8*)** %fnptr, align 8
+  %0 = load i32 (i8*)*, i32 (i8*)** %fnptr, align 8
+  %call1 = call i32 %0(i8* noundef null) #0
+  call void @notsetjmp()
+  ret void
+}
+
+declare i32 @setjmp(i8* noundef) #0
+declare void @notsetjmp()
+
+attributes #0 = { returns_twice }
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"branch-target-enforcement", i32 1}
Index: llvm/test/CodeGen/AArch64/setjmp-bti-outliner.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/setjmp-bti-outliner.ll
@@ -0,0 +1,83 @@
+; RUN: llc -mtriple=aarch64-none-linux-gnu -enable-machine-outliner < %s | FileCheck %s --check-prefix=BTI
+; RUN: llc -mtriple=aarch64-none-linux-gnu -global-isel -enable-machine-outliner < %s | \
+; RUN: FileCheck %s --check-prefix=BTI
+; RUN: llc -mtriple=aarch64-none-linux-gnu -fast-isel -enable-machine-outliner < %s | \
+; RUN: FileCheck %s --check-prefix=BTI
+; RUN: llc -mtriple=aarch64-none-linux-gnu -enable-machine-outliner -mattr=+no-bti-at-return-twice < %s | \
+; RUN: FileCheck %s --check-prefix=NOBTI
+; RUN: llc -mtriple=aarch64-none-linux-gnu -global-isel -enable-machine-outliner -mattr=+no-bti-at-return-twice < %s | \
+; RUN: FileCheck %s --check-prefix=NOBTI
+; RUN: llc -mtriple=aarch64-none-linux-gnu -fast-isel -enable-machine-outliner -mattr=+no-bti-at-return-twice < %s | \
+; RUN: FileCheck %s --check-prefix=NOBTI
+
+; Check that the outliner does not split up the call to setjmp and the 

[PATCH] D121694: [clang][dataflow] Allow disabling built-in transfer functions for CFG terminators

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

xazax.hun added a comment.

In D121694#3382587 , @ymandel wrote:

> In D121694#3382473 , @xazax.hun 
> wrote:
>
>> The change itself looks good. But out of curiosity, could you give me an 
>> example when we do not want to use the builtin transfer functions?
>
> Sure! Pretty much any plain-vanilla dataflow analysis that sticks to its own 
> lattice and doesn't care about the environment. The demo constant-propagation 
> analyses are like this, but we have additional real analyses using the 
> framework in this way. Examples include an analysis to detect raw pointers 
> that could be unique pointers and one that detects missed opportunies to use 
> `std::move`.

Makes sense! Although, I wonder if we would want an alternative API where the 
transfer function would not even get Env as an argument. So users could pick 
the right class to derive from for their needs and the decision whether use the 
built-in transfer functions would be compile time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121694

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

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think we're pretty close; it looks like there's still one unresolved 
conversation about template instantiation needs and whether we can remove that 
code or not.




Comment at: clang/include/clang/AST/TypeLoc.h:923
+  void initializeLocal(ASTContext &Context, SourceLocation loc) {}
+
+  QualType getInnerType() const { return getTypePtr()->getWrappedType(); }

yonghong-song wrote:
> yonghong-song wrote:
> > aaron.ballman wrote:
> > > yonghong-song wrote:
> > > > aaron.ballman wrote:
> > > > > Do we also need something like this? 
> > > > > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/TypeLoc.h#L1187
> > > > I didn't see getLocalDataSize() in AttributedTypeLoc so it is not 
> > > > needed for AttributedTypeLoc. BTFTagAttributedTypeLoc usage is very 
> > > > similar to AttributedTypeLoc, so I think we are fine here.
> > > The main difference is that `AttributedLocInfo` has a member and 
> > > `BTFTagAttributedLocInfo` is empty. This is why I think it's closer to an 
> > > `AdjustedLocInfo` object which also is an empty struct.
> > I think adding getLocalDataSize() return 0 should be OK. But it looks like 
> > we don't need to do it, at least for BTFTagAttributedTypeLoc. The following 
> > are some details.
> > 
> > The parent class of BTFTagAttributedTypeLoc is ConcreteTypeLoc, which 
> > contains:
> > 
> >   unsigned getLocalDataSize() const {
> > unsigned size = sizeof(LocalData);
> > unsigned extraAlign = asDerived()->getExtraLocalDataAlignment();
> > size = llvm::alignTo(size, extraAlign);
> > size += asDerived()->getExtraLocalDataSize();
> > return size;
> >   }
> > 
> >   unsigned getExtraLocalDataSize() const {
> > return 0;
> >   }
> > 
> >   unsigned getExtraLocalDataAlignment() const {
> > return 1;
> >   } 
> > 
> > Manually calculating getLocalDataSize() can get the same return value 0. So 
> > I think it is okay not to define  getLocalDataSize in 
> > BTFTagAttributedTypeLoc. 
> > 
> > The AdjustedLocInfo seems having some inconsistency, at least based on 
> > comments:
> > 
> > struct AdjustedLocInfo {}; // Nothing.
> >   unsigned getLocalDataSize() const {
> > // sizeof(AdjustedLocInfo) is 1, but we don't need its address to be 
> > unique
> > // anyway.  TypeLocBuilder can't handle data sizes of 1.
> > return 0;  // No data.
> >   }
> > Maybe previously AdjustedLocInfo size is 1 and the implementation of 
> > getLocalDataSize() to set size to 0 explicitly in which case, the function 
> > is needed inside AdjustedTypeLoc. It might be needed now since 
> > AdjustedLocInfo size is 0.
> > 
> For
> > Maybe previously AdjustedLocInfo size is 1 and the implementation of 
> > getLocalDataSize() to set size to 0 explicitly in which case, the function 
> > is needed inside AdjustedTypeLoc. It might be needed now since 
> > AdjustedLocInfo size is 0.
> I mean "it might not be needed now since AdjustedLocInfo size is 0".
Ahh, I see why we get away with it then. Thank you!



Comment at: clang/lib/Sema/TreeTransform.h:6872
+  const BTFTagAttributedType *oldType = TL.getTypePtr();
+  StringRef Tag = oldType->getTag();
+  QualType modifiedType = getDerived().TransformType(TLB, TL.getModifiedLoc());

aaron.ballman wrote:
> yonghong-song wrote:
> > erichkeane wrote:
> > > Most of this tree-transform doesn't really do much, since this is a 'C' 
> > > only type, but otherwise we'd probably want to allow the tag itself to be 
> > > dependent.  
> > > 
> > > We still need this though, since there are other non-template 
> > > tree-transforms.
> > > 
> > > You also might need to make this not contain a `StringRef` based on the 
> > > serialization issues above.
> > I will try to do some experiment and simplify this. Indeed this is C and 
> > non templates are involved.
> > We still need this though, since there are other non-template 
> > tree-transforms.
> 
> Are we sure any of them can be hit for this new type? It'd be nice to keep 
> this out of the template instantiation bits if possible.
I think this may be the only unresolved conversation in the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120296

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

[PATCH] D120527: [OpaquePtr][AArch64] Use elementtype on ldxr/stxr

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

In D120527#3381835 , @nikic wrote:

> @aeubanks Do you plan to take care of the corresponding arm intrinsics as 
> well?

yes I'll do those


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120527

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

[PATCH] D121709: [NFC][AIX] Disable precompiled module file test on AIX

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan created this revision.
Herald added a project: All.
Jake-Egan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121709

Files:
  clang/test/ClangScanDeps/modules-no-undeclared-includes.c


Index: clang/test/ClangScanDeps/modules-no-undeclared-includes.c
===
--- clang/test/ClangScanDeps/modules-no-undeclared-includes.c
+++ clang/test/ClangScanDeps/modules-no-undeclared-includes.c
@@ -1,3 +1,7 @@
+// Unsupported on AIX because we don't support the requisite "__clangast"
+// section in XCOFF yet.
+// UNSUPPORTED: aix
+
 // RUN: rm -rf %t && mkdir %t
 // RUN: split-file %s %t
 


Index: clang/test/ClangScanDeps/modules-no-undeclared-includes.c
===
--- clang/test/ClangScanDeps/modules-no-undeclared-includes.c
+++ clang/test/ClangScanDeps/modules-no-undeclared-includes.c
@@ -1,3 +1,7 @@
+// Unsupported on AIX because we don't support the requisite "__clangast"
+// section in XCOFF yet.
+// UNSUPPORTED: aix
+
 // RUN: rm -rf %t && mkdir %t
 // RUN: split-file %s %t
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 415460.
jhuber6 added a comment.

Accidentally clang formatted an entire file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cuda-openmp-driver.cu

Index: clang/test/Driver/cuda-openmp-driver.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-openmp-driver.cu
@@ -0,0 +1,16 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings -fgpu-rdc \
+// RUN:-foffload-new-driver --offload-arch=sm_35 --offload-arch=sm_70 %s 2>&1 \
+// RUN: | FileCheck -check-prefix CHECK %s
+
+// CHECK: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[PTX_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[PTX_SM_35]]"], output: "[[CUBIN_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Linker", inputs: ["[[CUBIN_SM_35]]", "[[PTX_SM_35]]"], output: "[[FATBIN_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]"], output: "[[PTX_SM_70:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[PTX_SM_70:.+]]"], output: "[[CUBIN_SM_70:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Linker", inputs: ["[[CUBIN_SM_70]]", "[[PTX_SM_70:.+]]"], output: "[[FATBIN_SM_70:.+]]"
+// CHECK: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT]]", "[[FATBIN_SM_35]]", "[[FATBIN_SM_70]]"], output: "[[HOST_OBJ:.+]]"
+// CHECK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4391,6 +4391,7 @@
   // one input.
   bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
   bool IsCudaDevice = JA.isDeviceOffloading(Action::OFK_Cuda);
+  bool IsCudaHost = JA.isHostOffloading(Action::OFK_Cuda);
   bool IsHIP = JA.isOffloading(Action::OFK_HIP);
   bool IsHIPDevice = JA.isDeviceOffloading(Action::OFK_HIP);
   bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
@@ -4414,6 +4415,7 @@
 
   InputInfoList ModuleHeaderInputs;
   InputInfoList OpenMPHostInputs;
+  InputInfoList CudaHostInputs;
   const InputInfo *CudaDeviceInput = nullptr;
   const InputInfo *OpenMPDeviceInput = nullptr;
   for (const InputInfo &I : Inputs) {
@@ -4428,6 +4430,9 @@
 << types::getTypeName(Expected);
   }
   ModuleHeaderInputs.push_back(I);
+} else if (IsOpenMPDevice && !OpenMPDeviceInput) {
+  CudaDeviceInput = &I;
+  CudaHostInputs.push_back(I);
 } else if ((IsCuda || IsHIP) && !CudaDeviceInput) {
   CudaDeviceInput = &I;
 } else if (IsOpenMPDevice && !OpenMPDeviceInput) {
@@ -6982,6 +6987,20 @@
   TC->getTripleString() + "." +
   TCArgs.getLastArgValue(options::OPT_march_EQ) + "." + InputName));
 }
+  } else if (IsCudaHost && !CudaHostInputs.empty()) {
+const ToolChain *TC = C.getSingleOffloadToolChain();
+for (const auto &InputFile : CudaHostInputs) {
+  assert(InputFile.isFilename() && "Offloading requires a filename");
+  StringRef File =
+  C.getArgs().MakeArgString(TC->getInputFilename(InputFile));
+  StringRef InputName = Clang::getBaseInputStem(Args, Inputs);
+  // The CUDA toolchain should have a bound arch appended to the filename.
+  StringRef Arch = File.rsplit(".").first.rsplit('-').second;
+  CmdArgs.push_back(Args.MakeArgString(
+  "-fembed-offload-object=" + File + "," +
+  Action::GetOffloadKindName(Action::OFK_Cuda) + "." +
+  TC->getTripleString() + "." + Arch + "." + InputName));
+}
   }
 
   if (Triple.isAMDGPU()) {
@@ -8234,14 +8253,17 @@
   ArgStringList CmdArgs;
 
   // Pass the CUDA path to the linker wrapper tool.
-  for (auto &I : llvm::make_range(OpenMPTCRange.first, OpenMPTCRange.second)) {
-const ToolChain *TC = I.second;
-if (TC->getTriple().isNVPTX()) {
-  CudaInstallationDetector CudaInstallation(D, TheTriple, Args);
-  if (CudaInstallation.isValid())
-CmdArgs.push_back(Args.MakeArgString(
-"--cuda-path=" + CudaInstallation.getInstallPath()));
-  break;
+  for (Action::OffloadKind Kind : {Action::OFK_Cuda, Action::OFK_OpenMP}) {
+auto TCRange = C.getOffloadToolChains(Kind);
+for (auto &I : llvm::make_range(TCRange.first, TCRange.second)) {
+  const ToolChain *TC = I.second;
+  if (TC->getTriple().isNVPTX()) {
+CudaInstallationDetector CudaInstallation(D, TheTriple, Args);
+if (CudaInstallation.isValid())
+  CmdArgs.pu

[PATCH] D111400: [Clang] Implement P2242R3

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 415464.
cor3ntin added a comment.

Fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaCXX/constant-expression-cxx2b.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1361,7 +1361,7 @@
 
   Non-literal variables (and labels and gotos) in constexpr functions
   https://wg21.link/P2242R3";>P2242R3
-  No
+  Clang 15
 
 
   Character encoding of diagnostic text
Index: clang/test/SemaCXX/constant-expression-cxx2b.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constant-expression-cxx2b.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify %s -fcxx-exceptions -triple=x86_64-linux-gnu -Wpre-c++2b-compat
+
+constexpr int f(int n) {  // expected-error {{constexpr function never produces a constant expression}}
+  static const int m = n; // expected-warning {{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+  // expected-note {{control flows through the declaration of a static variable}}
+  return m;
+}
+constexpr int g(int n) {// expected-error {{constexpr function never produces a constant expression}}
+  thread_local const int m = n; // expected-warning {{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+// expected-note {{control flows through the declaration of a thread_local variable}}
+  return m;
+}
+
+constexpr int h(int n) {  // expected-error {{constexpr function never produces a constant expression}}
+  static const int m = n; // expected-warning {{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+  // expected-note {{control flows through the declaration of a static variable}}
+  return &m - &m;
+}
+constexpr int i(int n) {// expected-error {{constexpr function never produces a constant expression}}
+  thread_local const int m = n; // expected-warning {{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+// expected-note {{control flows through the declaration of a thread_local variable}}
+  return &m - &m;
+}
+
+constexpr int j(int n) {
+  if (!n)
+return 0;
+  static const int m = n; // expected-warning{{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}}
+  return m;
+}
+
+constexpr int k(int n) {
+  if (!n)
+return 0;
+  thread_local const int m = n; // expected-warning{{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+
+  return m;
+}
+
+constexpr int j_evaluated(int n) {
+  if (!n)
+return 0;
+  static const int m = n; // expected-warning{{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+  // expected-note {{control flows through the declaration of a static variable}}
+  return m;
+}
+
+constexpr int je = j_evaluated(1); // expected-error {{constexpr variable 'je' must be initialized by a constant expression}}  \
+   // expected-note {{in call}}
+
+constexpr int k_evaluated(int n) {
+  if (!n)
+return 0;
+  thread_local const int m = n; // expected-warning{{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+// expected-note {{control flows through the declaration of a thread_local variable}}
+
+  return m;
+}
+
+constexpr int ke = k_evaluated(1); // expected-error {{constexpr variable 'ke' must be initialized by a constant expression}} \
+   // expected-note {{in call}}
+
+namespace eval_goto {
+
+constexpr int f(int x) {
+  if (x) {
+return 0;
+  } else {
+goto test; // expected-note {{subexpression not valid in a constant expression}} \
+// expected-warning {{use of this statement in a constexpr function is incompatible with C++ s

[PATCH] D121694: [clang][dataflow] Allow disabling built-in transfer functions for CFG terminators

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D121694#3382683 , @xazax.hun wrote:

> In D121694#3382587 , @ymandel wrote:
>
>> In D121694#3382473 , @xazax.hun 
>> wrote:
>>
>>> The change itself looks good. But out of curiosity, could you give me an 
>>> example when we do not want to use the builtin transfer functions?
>>
>> Sure! Pretty much any plain-vanilla dataflow analysis that sticks to its own 
>> lattice and doesn't care about the environment. The demo 
>> constant-propagation analyses are like this, but we have additional real 
>> analyses using the framework in this way. Examples include an analysis to 
>> detect raw pointers that could be unique pointers and one that detects 
>> missed opportunies to use `std::move`.
>
> Makes sense! Although, I wonder if we would want an alternative API where the 
> transfer function would not even get Env as an argument. So users could pick 
> the right class to derive from for their needs and the decision whether use 
> the built-in transfer functions would be compile time.

That would make sense. We definitely don't want to stick w/ the current setup. 
But, we've been thinking that we can split out `Env` and its transfer functions 
as a standalone "model" that users can choose to compose with their own (more 
specialized) model. I'm planning to send a patch shortly with an initial API 
for "models" (basically, a virtual API corresponding to `DataflowAnalysis` as 
of now).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121694

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

[PATCH] D111587: re-land: [clang] Fix absolute file paths with -fdebug-prefix-map

[Keith Smiley via Phabricator via cfe-commits]

keith added a comment.

I actually mean dwarf 6, which appears to be partially implemented according to 
https://lists.llvm.org/pipermail/llvm-dev/2020-April/141055.html

I discovered the issue from the failed tests on 
https://reviews.llvm.org/D113718 where you can see the test output contains a 
checksum that otherwise doesn't appear. Passing `-dwarf-version=6` reproduces 
the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111587

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

[PATCH] D111587: re-land: [clang] Fix absolute file paths with -fdebug-prefix-map

[Keith Smiley via Phabricator via cfe-commits]

keith added inline comments.



Comment at: clang/test/Modules/module-debuginfo-prefix.m:24
 
-// Dir should always be empty, but on Windows we can't recognize /var
-// as being an absolute path.
-// CHECK: !DIFile(filename: "/OVERRIDE/DebugObjC.h", directory: 
"{{()|(.*:.*)}}")
+// CHECK: !DIFile(filename: "{{/|.:}}OVERRIDE{{/|}}DebugObjC.h", 
directory: "")

probinson wrote:
> Does this want to be 
> `"%{fs-src-root}OVERRIDE%{fs-sep}DebugObjC.h"` ?
That can only be used in the RUN invocations, but unfortunately regardless it 
could be used here because of the escaped backslashes, so it requires a regex 
like this instead 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111587

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

[PATCH] D121712: [clangd] Track time spent in filesystem ops during preamble builds

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz created this revision.
adamcz added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman, javed.absar.
Herald added a project: All.
adamcz requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

In some deployments, for example when running on FUSE or using some
network-based VFS implementation, the filesystem operations might add up
to a significant fraction of preamble build time. This change allows us
to track time spent in FS operations to better understand the problem.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121712

Files:
  clang-tools-extra/clangd/FS.cpp
  clang-tools-extra/clangd/FS.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp

Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -101,7 +101,8 @@
   auto ModuleCacheDeleter = llvm::make_scope_exit(
   std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath));
   return clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs,
-  /*StoreInMemory=*/true, PreambleCallback);
+  /*StoreInMemory=*/true, /*Stats=*/nullptr,
+  PreambleCallback);
 }
 
 ParsedAST TestTU::build() const {
@@ -117,6 +118,7 @@
 
   auto Preamble = clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs,
/*StoreInMemory=*/true,
+   /*Stats=*/nullptr,
/*PreambleCallback=*/nullptr);
   auto AST = ParsedAST::build(testPath(Filename), Inputs, std::move(CI),
   Diags.take(), Preamble);
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -172,8 +172,9 @@
   TU.AdditionalFiles["b.h"] = "";
   TU.AdditionalFiles["c.h"] = "";
   auto PI = TU.inputs(FS);
-  auto BaselinePreamble = buildPreamble(
-  TU.Filename, *buildCompilerInvocation(PI, Diags), PI, true, nullptr);
+  auto BaselinePreamble =
+  buildPreamble(TU.Filename, *buildCompilerInvocation(PI, Diags), PI, true,
+/*Stats=*/nullptr, nullptr);
   // We drop c.h from modified and add a new header. Since the latter is patched
   // we should only get a.h in preamble includes.
   TU.Code = R"cpp(
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -494,8 +494,8 @@
   MockFS FS;
   auto Inputs = TU.inputs(FS);
   auto CI = buildCompilerInvocation(Inputs, Diags);
-  auto EmptyPreamble =
-  buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, nullptr);
+  auto EmptyPreamble = buildPreamble(testPath("foo.cpp"), *CI, Inputs, true,
+ /*Stats=*/nullptr, nullptr);
   ASSERT_TRUE(EmptyPreamble);
   EXPECT_THAT(EmptyPreamble->Includes.MainFileIncludes, IsEmpty());
 
@@ -537,8 +537,8 @@
   MockFS FS;
   auto Inputs = TU.inputs(FS);
   auto CI = buildCompilerInvocation(Inputs, Diags);
-  auto BaselinePreamble =
-  buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, nullptr);
+  auto BaselinePreamble = buildPreamble(testPath("foo.cpp"), *CI, Inputs, true,
+/*Stats=*/nullptr, nullptr);
   ASSERT_TRUE(BaselinePreamble);
   EXPECT_THAT(BaselinePreamble->Includes.MainFileIncludes,
   ElementsAre(testing::Field(&Inclusion::Written, "")));
Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -307,6 +307,7 @@
   bool IndexUpdated = false;
   buildPreamble(FooCpp, *CI, PI,
 /*StoreInMemory=*/true,
+/*Stats=*/nullptr,
 [&](ASTContext &Ctx, Preprocessor &PP,
 const CanonicalIncludes &CanonIncludes) {
   EXPECT_FALSE(IndexUpdated)
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

[PATCH] D121245: [clang][parser] Allow GNU attributes before namespace identifier

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX updated this revision to Diff 415470.
ArcsinX added a comment.

Update release notes
Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121245

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Parser/namespace-attributes.cpp

Index: clang/test/Parser/namespace-attributes.cpp
===
--- /dev/null
+++ clang/test/Parser/namespace-attributes.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+namespace __attribute__(()) A
+{
+}
+
+namespace A __attribute__(())
+{
+}
+
+namespace __attribute__(()) [[]] A
+{
+}
+
+namespace [[]] __attribute__(()) A
+{
+}
+
+namespace A __attribute__(()) [[]]
+{
+}
+
+namespace A [[]] __attribute__(())
+{
+}
+
+namespace [[]] A __attribute__(())
+{
+}
+
+namespace __attribute__(()) A [[]]
+{
+}
+
+namespace A::B __attribute__(()) // expected-error{{attributes cannot be specified on a nested namespace definition}}
+{
+}
+
+namespace __attribute__(()) A::B // expected-error{{attributes cannot be specified on a nested namespace definition}}
+{
+}
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -74,15 +74,27 @@
   SourceLocation FirstNestedInlineLoc;
 
   ParsedAttributesWithRange attrs(AttrFactory);
-  SourceLocation attrLoc;
-  if (getLangOpts().CPlusPlus11 && isCXX11AttributeSpecifier()) {
-Diag(Tok.getLocation(), getLangOpts().CPlusPlus17
-? diag::warn_cxx14_compat_ns_enum_attribute
-: diag::ext_ns_enum_attribute)
-  << 0 /*namespace*/;
-attrLoc = Tok.getLocation();
-ParseCXX11Attributes(attrs);
-  }
+
+  auto ReadAttributes = [&] {
+bool MoreToParse;
+do {
+  MoreToParse = false;
+  if (Tok.is(tok::kw___attribute)) {
+ParseGNUAttributes(attrs);
+MoreToParse = true;
+  }
+  if (getLangOpts().CPlusPlus11 && isCXX11AttributeSpecifier()) {
+Diag(Tok.getLocation(), getLangOpts().CPlusPlus17
+? diag::warn_cxx14_compat_ns_enum_attribute
+: diag::ext_ns_enum_attribute)
+<< 0 /*namespace*/;
+ParseCXX11Attributes(attrs);
+MoreToParse = true;
+  }
+} while (MoreToParse);
+  };
+
+  ReadAttributes();
 
   if (Tok.is(tok::identifier)) {
 Ident = Tok.getIdentifierInfo();
@@ -108,16 +120,14 @@
 }
   }
 
+  ReadAttributes();
+
+  SourceLocation attrLoc = attrs.Range.getBegin();
+
   // A nested namespace definition cannot have attributes.
   if (!ExtraNSs.empty() && attrLoc.isValid())
 Diag(attrLoc, diag::err_unexpected_nested_namespace_attribute);
 
-  // Read label attributes, if present.
-  if (Tok.is(tok::kw___attribute)) {
-attrLoc = Tok.getLocation();
-ParseGNUAttributes(attrs);
-  }
-
   if (Tok.is(tok::equal)) {
 if (!Ident) {
   Diag(Tok, diag::err_expected) << tok::identifier;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -94,6 +94,11 @@
   locations a declaration attribute may appear.
   This fixes `Issue 53805 `_.
 
+- Improved namespace attributes handling:
+  - Handle GNU attributes before a namespace identifier and subsequent
+attributes of different kinds.
+  - Emit error on GNU attributes for a nested namespace definition.
+
 Windows Support
 ---
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88905: [Clang] Allow "ext_vector_type" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll added a comment.

In D88905#3382309 , @aaron.ballman 
wrote:

> Thanks! Sema bits now LGTM

Awesome! Who would be a good fit for the codegen parts of this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88905

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

[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2733
+/// \endcode
+AST_MATCHER_P(AttributedStmt, isAttr, attr::Kind, AttrKind) {
+  return llvm::any_of(Node.getAttrs(),

ajohnson-uoregon wrote:
> aaron.ballman wrote:
> > sammccall wrote:
> > > This definitely seems more like `hasAttr` than `isAttr` to me. An 
> > > AttributedStmt *is* the (sugar) statement, and *has* the attribute(s).
> > > 
> > > Maybe rather `describesAttr` so people don't confuse this for one that 
> > > walks up, though?
> > Good point on the `isAttr` name!
> > 
> > I'm not sure `describeAttr` works (the statement doesn't describe 
> > attributes, it... has/contains/uses the attribute). Maybe.. 
> > `specifiesAttr()`?
> > 
> > Or are we making this harder than it needs to be and we should use 
> > `hasAttr()` for parity with other things that have attributes associated 
> > with them?
> Yeah I wasn't sure on the name, I just picked something that wouldn't make me 
> figure out polymorphic matcher declarations because this was the first AST 
> matcher I'd written. :)  
> 
> I like `containsAttr()` or `specifiesAttr()`, since it could have multiple 
> attributes. `hasAttr()` makes the most sense to me though. If y'all think we 
> should go with that, the new `hasAttr()` definition would look something like 
> this, right? (To make sure I do in fact understand polymorphic matchers.)
> 
> 
> ```
> AST_POLYMORPHIC_MATCHER_P(
> hasAttr,
> AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, AttributedStmt),
> attr::Kind, AttrKind) {
>   return llvm::any_of(Node.getAttrs(),
>   [&](const Attr *A) { return A->getKind() == AttrKind; 
> });
> }
> ```
> 
> Original `hasAttr()` for reference:
> ```
> AST_MATCHER_P(Decl, hasAttr, attr::Kind, AttrKind) {
>   for (const auto *Attr : Node.attrs()) {
> if (Attr->getKind() == AttrKind)
>   return true;
>   }
>   return false;
> }
> ```
> I like `containsAttr()` or `specifiesAttr()`, since it could have multiple 
> attributes. 

The same is true for declarations; they can have multiple attributes. Which is 
an interesting test case for us to add for statements:
```
int foo();

int main() {
  [[clang::nomerge]] [[likely]] return foo();
}
```
We should verify we can find both of those attributes on the same 
`attributedStmt()` (e.g., you don't have to do something like 
`attributedStmt(attributedStmt(...))` to match.)

As for the name, I think `hasAttr()` is fine; if it causes awkwardness in the 
future when we change the AST in Clang, we'll have to fix it up then, but that 
awkwardness will already be there for `attributedStmt()` itself.

> hasAttr() makes the most sense to me though. If y'all think we should go with 
> that, the new hasAttr() definition would look something like this, right?

That looks about like what I'd expect, yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

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

[PATCH] D121245: [clang][parser] Allow GNU attributes before namespace identifier

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D121245#3380346 , @aaron.ballman 
wrote:

> LGTM! Might be worth adding a release note for it (does this close any bugs 
> in the bug database? If so, it'd be worth mentioning those in the commit 
> message and release note).

Thank you for review.

I failed to find any opened bugs related to this. I'm not sure how detailed 
information about this patch should be in release notes. Can you please take a 
look at `ReleaseNotes.rst` modifications?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121245

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

[PATCH] D88905: [Clang] Allow "ext_vector_type" applied to Booleans

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D88905#3382881 , @simoll wrote:

> In D88905#3382309 , @aaron.ballman 
> wrote:
>
>> Thanks! Sema bits now LGTM
>
> Awesome! Who would be a good fit for the codegen parts of this patch?

Ping @craig.topper and @erichkeane as they both know CodeGen better than I do, 
or can add other reviewers to cover those bits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88905

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

[PATCH] D121245: [clang][parser] Allow GNU attributes before namespace identifier

[Aaron Ballman via Phabricator via cfe-commits]

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

In D121245#3382887 , @ArcsinX wrote:

> In D121245#3380346 , @aaron.ballman 
> wrote:
>
>> LGTM! Might be worth adding a release note for it (does this close any bugs 
>> in the bug database? If so, it'd be worth mentioning those in the commit 
>> message and release note).
>
> Thank you for review.
>
> I failed to find any opened bugs related to this. I'm not sure how detailed 
> information about this patch should be in release notes. Can you please take 
> a look at `ReleaseNotes.rst` modifications?

The release notes look great to me, thank you for adding them!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121245

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

[PATCH] D121715: [Clang] Fix an unused-but-set-variable warning with compond assignment

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song created this revision.
yonghong-song added a reviewer: eli.friedman.
Herald added a project: All.
yonghong-song requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For the following code,

  void test() {
  int j = 0;
  for (int i = 0; i < 1000; i++)
  j += 1;
  return;
  } 

If compiled with

  clang -g -Wall -Werror -S -emit-llvm test.c

we will see the following error:

  test.c:2:6: error: variable 'j' set but not used 
[-Werror,-Wunused-but-set-variable]
  int j = 0;
  ^   

This is not quite right since 'j' is indeed used due to '+=' operator.
gcc doesn't emit error either in this case.
Also if we change 'j += 1' to 'j++', the warning will disappear too 
with latest clang.

Our original use case is to use 'volatile int j = 0' with a loop like
in the above to burn cpu cycles so we could test certain kernel features.
The compilatin error will show up regardless of whether 'volatile' key 
word is used or not.

To fix the issue, in function MaybeDecrementCount(), if the 
operator is a compund assignment (i.e., +=, -=, etc.), we should
not decrement count for RefsMinusAssignments since here we have
both a reference and an assignment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121715

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/Sema/warn-unused-but-set-variables.c


Index: clang/test/Sema/warn-unused-but-set-variables.c
===
--- clang/test/Sema/warn-unused-but-set-variables.c
+++ clang/test/Sema/warn-unused-but-set-variables.c
@@ -59,3 +59,10 @@
   __attribute__((__cleanup__(for_cleanup))) int x;
   x = 5;
 }
+
+void f4() {
+  int j = 0;
+  for (int i = 0; i < 1000; i++)
+j += 1;
+  return;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7927,7 +7927,7 @@
 BO->getRHS()->getType()->isDependentType()) {
   if (BO->getOpcode() != BO_Assign)
 return;
-} else if (!BO->isAssignmentOp())
+} else if (!BO->isAssignmentOp() || BO->isCompoundAssignmentOp())
   return;
 LHS = dyn_cast(BO->getLHS());
   } else if (CXXOperatorCallExpr *COCE = dyn_cast(E)) {


Index: clang/test/Sema/warn-unused-but-set-variables.c
===
--- clang/test/Sema/warn-unused-but-set-variables.c
+++ clang/test/Sema/warn-unused-but-set-variables.c
@@ -59,3 +59,10 @@
   __attribute__((__cleanup__(for_cleanup))) int x;
   x = 5;
 }
+
+void f4() {
+  int j = 0;
+  for (int i = 0; i < 1000; i++)
+j += 1;
+  return;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7927,7 +7927,7 @@
 BO->getRHS()->getType()->isDependentType()) {
   if (BO->getOpcode() != BO_Assign)
 return;
-} else if (!BO->isAssignmentOp())
+} else if (!BO->isAssignmentOp() || BO->isCompoundAssignmentOp())
   return;
 LHS = dyn_cast(BO->getLHS());
   } else if (CXXOperatorCallExpr *COCE = dyn_cast(E)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Richard via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:47
+CRLF,
+CRLFCR,
+  };

aaron.ballman wrote:
> I'm a bit confused by this one as this is not a valid line ending (it's 
> either three valid line endings or two valid line endings, depending on how 
> you look at it). Can you explain why this is needed?
It's a state machine, where the states are named for what we've seen so far and 
we're looking for //two// consecutive line endings, not just one.  Does it make 
sense now?



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:83-84
+  const char *End = Begin + Token.getLength();
+  return std::none_of(
+  Begin, End, [](char C) { return C == '.' || std::toupper(C) == 'E'; });
+}

aaron.ballman wrote:
> Won't this cause a problem for hex literals like `0xE`?
Good catch, I'll add a test.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:106
+  int ConditionScopes;
+  unsigned int LastLine;
+  IncludeGuard GuardScanner;

aaron.ballman wrote:
> Maybe not worth worrying about, but should this be a `uint64_t` to handle 
> massive source files?
I use the type returned by getSpellingLineNumber.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:315
+  if (Info->isFunctionLike() || Info->isBuiltinMacro() ||
+  Info->tokens().empty() || Info->tokens().size() > 2)
+return;

aaron.ballman wrote:
> We don't seem to have any tests for literal suffixes and I *think* those will 
> show up here as additional tokens after the literal. e.g., `#define FOO 
> +1ULL`, so I think the size check here may be a problem.
On an earlier iteration I had some tests for literals with suffixes and the 
suffix is included in the literal token.  I can add some test cases just to 
make it clear what the behavior is when they are encountered.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:318
+
+  // It can be +Lit, -Lit or just Lit.
+  Token Tok = Info->tokens().front();

aaron.ballman wrote:
> It's worth pointing out that both of these are expressions that operate on a 
> literal, and if we're going to allow expressions that operator on a literal, 
> why only these two? e.g. why not allow `#define FOO ~0U` or `#define BAR FOO 
> + 1`? Someday (not today), it would be nice for this to work on any 
> expression that's a valid constant expression.
A later enhancement can generalize to literal expressions (which are valid 
initializers for an enumeration), but I wanted to cover the most common case of 
simple negative integers in this first pass.



Comment at: clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp:88-92
+CheckFactories.registerCheck(
+"modernize-use-equals-default");
 CheckFactories.registerCheck(
 "modernize-use-equals-delete");
+CheckFactories.registerCheck("modernize-use-nodiscard");

aaron.ballman wrote:
> Unrelated formatting changes? (Feel free to land separately)
Probably, I just routinely clang-format the whole file instead of just isolated 
changes.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp:67-68
+
+// Undefining a macro invalidates adjacent macros
+// from being considered as an enum.
+#define REMOVED1 1

aaron.ballman wrote:
> What about an #undef that's not adjacent to any macros? e.g.,
> ```
> #define FOO 1
> #define BAR 2
> #define BAZ 3
> 
> int i = 12;
> 
> #if defined(FROBBLE)
> #undef FOO
> #endif
> ```
> I'm worried that perhaps other code elsewhere will be checking `defined(FOO)` 
> perhaps in cases conditionally compiled away, and switching `FOO` to be an 
> enum constant will break other configurations. To be honest, I'm a bit 
> worried about that for all of the transformations here... and I don't know a 
> good way to address that aside from "don't use the check". It'd be 
> interesting to know what kind of false positive rate we have for the fixes if 
> we ran it over a large corpus of code.
Yeah, the problem arises whenever you make any changes to a header file.  Did 
you also change all translation units that include the header?  What about 
conditionally compiled code that was "off" in the translation unit for the 
automated change?  Currently, we don't have a way of analyzing a group of 
translation units together for a cohesive change, nor do we have any way of 
inspecting more deeply into conditionally compiled code.  Addressing those 
concerns is beyond the scope of this check (or any clang-tidy check) as it 
involves improvements to the entire infrastructure.

However, I think it is worth noting in the documentation about possible 
caveats.  I think the way clang

[PATCH] D121715: [Clang] Fix an unused-but-set-variable warning with compond assignment

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

@eli.friedman Please feel free adding other reviewers if needed. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121715

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

[PATCH] D88905: [Clang] Allow "ext_vector_type" applied to Booleans

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Everything in the CodeGen looks fine, but I didn't see anything for constant 
evaluation?  Can you ensure that the ExprConstant evaluation for Vectors still 
works for this type (or, exclude it from participating in constant expressions 
entirely)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88905

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

[PATCH] D120187: [clang-tidy] Allow newline characters as separators for checks in Clang-Tidy configurations

[Danny Mösch via Phabricator via cfe-commits]

SimplyDanny added a comment.

In D120187#3375610 , @aaron.ballman 
wrote:

> LGTM, but please wait a day or two before landing in case @njames93 has 
> further concerns.

Would you merge the changes, @aaron.ballman? Seems like there are no objections 
from others. Please use the commit details mentioned here 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120187

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

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:6872
+  const BTFTagAttributedType *oldType = TL.getTypePtr();
+  StringRef Tag = oldType->getTag();
+  QualType modifiedType = getDerived().TransformType(TLB, TL.getModifiedLoc());

aaron.ballman wrote:
> aaron.ballman wrote:
> > yonghong-song wrote:
> > > erichkeane wrote:
> > > > Most of this tree-transform doesn't really do much, since this is a 'C' 
> > > > only type, but otherwise we'd probably want to allow the tag itself to 
> > > > be dependent.  
> > > > 
> > > > We still need this though, since there are other non-template 
> > > > tree-transforms.
> > > > 
> > > > You also might need to make this not contain a `StringRef` based on the 
> > > > serialization issues above.
> > > I will try to do some experiment and simplify this. Indeed this is C and 
> > > non templates are involved.
> > > We still need this though, since there are other non-template 
> > > tree-transforms.
> > 
> > Are we sure any of them can be hit for this new type? It'd be nice to keep 
> > this out of the template instantiation bits if possible.
> I think this may be the only unresolved conversation in the review.
> Are we sure any of them can be hit for this new type? It'd be nice to keep 
> this out of the
> template instantiation bits if possible.

Actually I am not sure. But looks like we have to have the function implemented 
due to
'automatic code generation'. Let me take a look. If we don't need it since the 
attribute for C
only. The implementation can be just a unreachable failure or somehow I will 
see whether I can
tweak the auto code generation to avoid instantiating this function. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120296

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

[PATCH] D111400: [Clang] Implement P2242R3

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

@aaron.ballman @cor3ntin, are we confident that testing the non-lambda cases is 
sufficient to cover the lambda cases as well?

I suggest using a pattern such as:

  int (*test_cxx2b_constexpr_label_in_body())() {
auto qq = []() {
  label: return 42;
};
const int x = qq();
auto ff = [] { return x; }; // passes in C++2b; error in C++20
return ff;
  }

For each of the cases.




Comment at: clang/lib/AST/ExprConstant.cpp:5010
+  // through a declaration of a variable with static or thread storage 
duration.
+  if (VD->isLocalVarDecl() && !VD->isConstexpr() && VD->isStaticLocal()) {
+Info.CCEDiag(VD->getLocation(), diag::note_constexpr_static_local)

I don't see anything in the wording exempting `constexpr static` (although I 
suppose it makes sense to, but GCC does not make such an exemption in its C++2b 
mode). @cor3ntin, can you open a discussion on the reflector?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D121283: [Clang] Support multiple attributes in a single pragma

[Aaron Ballman via Phabricator via cfe-commits]

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

Aside from a minor testing nit, this LGTM. Thank you for the new functionality!




Comment at: clang/test/Parser/pragma-multiple-attributes.cpp:1-11
+// RUN: %clang_cc1 -Wno-pragma-clang-attribute -verify %s
+
+#pragma clang attribute push ([[clang::disable_tail_calls, noreturn]], 
apply_to = function)
+#pragma clang attribute pop
+
+#pragma clang attribute push (__attribute__((disable_tail_calls, 
annotate("test"))), apply_to = function)
+#pragma clang attribute pop

I think this entire test file can now be folded back into 
`pragma-attribute.cpp` (this way we don't have to pay the overhead of another 
execution of Clang to test the functionality).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121283

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

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:6872
+  const BTFTagAttributedType *oldType = TL.getTypePtr();
+  StringRef Tag = oldType->getTag();
+  QualType modifiedType = getDerived().TransformType(TLB, TL.getModifiedLoc());

yonghong-song wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > yonghong-song wrote:
> > > > erichkeane wrote:
> > > > > Most of this tree-transform doesn't really do much, since this is a 
> > > > > 'C' only type, but otherwise we'd probably want to allow the tag 
> > > > > itself to be dependent.  
> > > > > 
> > > > > We still need this though, since there are other non-template 
> > > > > tree-transforms.
> > > > > 
> > > > > You also might need to make this not contain a `StringRef` based on 
> > > > > the serialization issues above.
> > > > I will try to do some experiment and simplify this. Indeed this is C 
> > > > and non templates are involved.
> > > > We still need this though, since there are other non-template 
> > > > tree-transforms.
> > > 
> > > Are we sure any of them can be hit for this new type? It'd be nice to 
> > > keep this out of the template instantiation bits if possible.
> > I think this may be the only unresolved conversation in the review.
> > Are we sure any of them can be hit for this new type? It'd be nice to keep 
> > this out of the
> > template instantiation bits if possible.
> 
> Actually I am not sure. But looks like we have to have the function 
> implemented due to
> 'automatic code generation'. Let me take a look. If we don't need it since 
> the attribute for C
> only. The implementation can be just a unreachable failure or somehow I will 
> see whether I can
> tweak the auto code generation to avoid instantiating this function. 
If we need it, we need it. But the more we can remove (like replacing the 
implementation with an unreachable), the better, so thank you for looking into 
it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120296

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

[clang] 1c1a4b9 - [clang][driver] Emit a warning if -xc/-xc++ is after the last input file

[Yi Kong via cfe-commits]

Author: Yi Kong
Date: 2022-03-16T01:08:00+08:00
New Revision: 1c1a4b9556db8579f7428605ac2c351bddde9ad5

URL: 
https://github.com/llvm/llvm-project/commit/1c1a4b9556db8579f7428605ac2c351bddde9ad5
DIFF: 
https://github.com/llvm/llvm-project/commit/1c1a4b9556db8579f7428605ac2c351bddde9ad5.diff

LOG: [clang][driver] Emit a warning if -xc/-xc++ is after the last input file

This follows the same warning GCC produces.

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

Added: 
clang/test/Driver/x-args.c

Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/Driver.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index afedb37797e32..3394eefd738c4 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -365,6 +365,9 @@ def warn_drv_preprocessed_input_file_unused : Warning<
 def warn_drv_unused_argument : Warning<
   "argument unused during compilation: '%0'">,
   InGroup;
+def warn_drv_unused_x : Warning<
+  "‘-x %0’ after last input file has no effect">,
+  InGroup;
 def warn_drv_empty_joined_argument : Warning<
   "joined argument expects additional value: '%0'">,
   InGroup;

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 70822490e63b8..d7a5ca4f21988 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -2308,6 +2308,15 @@ void Driver::BuildInputs(const ToolChain &TC, 
DerivedArgList &Args,
 assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }
 
+  // Warn -x after last input file has no effect
+  {
+Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
+Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
+if (LastInputArg->getIndex() < LastXArg->getIndex()) {
+  Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+}
+  }
+
   for (Arg *A : Args) {
 if (A->getOption().getKind() == Option::InputClass) {
   const char *Value = A->getValue();

diff  --git a/clang/test/Driver/x-args.c b/clang/test/Driver/x-args.c
new file mode 100644
index 0..9b095fcd6c7f9
--- /dev/null
+++ b/clang/test/Driver/x-args.c
@@ -0,0 +1,7 @@
+// RUN: %clang -fsyntax-only -Werror -xc %s
+// RUN: %clang -fsyntax-only -Werror %s -xc %s
+
+// RUN: %clang -fsyntax-only %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
+// CHECK: ‘-x c++’ after last input file has no effect
\ No newline at end of file



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

[PATCH] D121683: Emit a warning if -xc/-xc++ is after the last input file

[Yi Kong via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1c1a4b9556db: [clang][driver] Emit a warning if -xc/-xc++ is 
after the last input file (authored by kongyi).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121683

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/x-args.c


Index: clang/test/Driver/x-args.c
===
--- /dev/null
+++ clang/test/Driver/x-args.c
@@ -0,0 +1,7 @@
+// RUN: %clang -fsyntax-only -Werror -xc %s
+// RUN: %clang -fsyntax-only -Werror %s -xc %s
+
+// RUN: %clang -fsyntax-only %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
+// CHECK: ‘-x c++’ after last input file has no effect
\ No newline at end of file
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2308,6 +2308,15 @@
 assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }
 
+  // Warn -x after last input file has no effect
+  {
+Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
+Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
+if (LastInputArg->getIndex() < LastXArg->getIndex()) {
+  Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+}
+  }
+
   for (Arg *A : Args) {
 if (A->getOption().getKind() == Option::InputClass) {
   const char *Value = A->getValue();
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -365,6 +365,9 @@
 def warn_drv_unused_argument : Warning<
   "argument unused during compilation: '%0'">,
   InGroup;
+def warn_drv_unused_x : Warning<
+  "‘-x %0’ after last input file has no effect">,
+  InGroup;
 def warn_drv_empty_joined_argument : Warning<
   "joined argument expects additional value: '%0'">,
   InGroup;


Index: clang/test/Driver/x-args.c
===
--- /dev/null
+++ clang/test/Driver/x-args.c
@@ -0,0 +1,7 @@
+// RUN: %clang -fsyntax-only -Werror -xc %s
+// RUN: %clang -fsyntax-only -Werror %s -xc %s
+
+// RUN: %clang -fsyntax-only %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
+// CHECK: ‘-x c++’ after last input file has no effect
\ No newline at end of file
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2308,6 +2308,15 @@
 assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }
 
+  // Warn -x after last input file has no effect
+  {
+Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
+Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
+if (LastInputArg->getIndex() < LastXArg->getIndex()) {
+  Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+}
+  }
+
   for (Arg *A : Args) {
 if (A->getOption().getKind() == Option::InputClass) {
   const char *Value = A->getValue();
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -365,6 +365,9 @@
 def warn_drv_unused_argument : Warning<
   "argument unused during compilation: '%0'">,
   InGroup;
+def warn_drv_unused_x : Warning<
+  "‘-x %0’ after last input file has no effect">,
+  InGroup;
 def warn_drv_empty_joined_argument : Warning<
   "joined argument expects additional value: '%0'">,
   InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk updated this revision to Diff 415487.
kwk added a comment.

Changed the strategy of how includes are sorted by increasing the priority if 
an include is of the "new" type: `@import Foundation;`. This will sort these 
includes after everything else just as requested in 
https://github.com/llvm/llvm-project/issues/38995.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

Files:
  clang/lib/Format/Format.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/unittests/Format/SortIncludesTest.cpp

Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -458,6 +458,39 @@
  "#include \"b.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, SupportAtImportLines) {
+  // Test from https://github.com/llvm/llvm-project/issues/38995
+  EXPECT_EQ("#import \"a.h\"\n"
+"#import \"b.h\"\n"
+"#import \"c.h\"\n"
+"#import \n"
+"@import Foundation;\n",
+sort("#import \"b.h\"\n"
+ "#import \"c.h\"\n"
+ "#import \n"
+ "@import Foundation;\n"
+ "#import \"a.h\"\n"));
+
+  // Slightly more complicated test that shows sorting in each priorities still
+  // works.
+  EXPECT_EQ("#import \"a.h\"\n"
+"#import \"b.h\"\n"
+"#import \"c.h\"\n"
+"#import \n"
+"@import Base;\n"
+"@import Foundation;\n"
+"@import base;\n"
+"@import foundation;\n",
+sort("#import \"b.h\"\n"
+ "#import \"c.h\"\n"
+ "@import Base;\n"
+ "#import \n"
+ "@import foundation;\n"
+ "@import Foundation;\n"
+ "@import base;\n"
+ "#import \"a.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, LeavesMainHeaderFirst) {
   Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
   EXPECT_EQ("#include \"llvm/a.h\"\n"
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -170,11 +170,11 @@
 }
 
 inline StringRef trimInclude(StringRef IncludeName) {
-  return IncludeName.trim("\"<>");
+  return IncludeName.trim("\"<>;");
 }
 
 const char IncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#][\t\ ]*(import|include)([^"]*("[^"]+")|[^<]*(<[^>]+>)|[\t\ ]*([^;]+;)))";
 
 // The filename of Path excluding extension.
 // Used to match implementation with headers, this differs from sys::path::stem:
@@ -290,10 +290,17 @@
   for (auto Line : Lines) {
 NextLineOffset = std::min(Code.size(), Offset + Line.size() + 1);
 if (IncludeRegex.match(Line, &Matches)) {
+  StringRef IncludeName;
+  for (int i=Matches.size()-1; i>0; i--) {
+if (!Matches[i].empty()) {
+  IncludeName = Matches[i];
+  break;
+}
+  }
   // If this is the last line without trailing newline, we need to make
   // sure we don't delete across the file boundary.
   addExistingInclude(
-  Include(Matches[2],
+  Include(IncludeName,
   tooling::Range(
   Offset, std::min(Line.size() + 1, Code.size() - Offset))),
   NextLineOffset);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2682,8 +2682,7 @@
 namespace {
 
 const char CppIncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
-
+R"(^[\t\ ]*[@#][\t\ ]*(import|include)([^"]*("[^"]+")|[^<]*(<[^>]+>)|[\t\ ]*([^;]+;)))";
 } // anonymous namespace
 
 tooling::Replacements sortCppIncludes(const FormatStyle &Style, StringRef Code,
@@ -2698,7 +2697,7 @@
   llvm::Regex IncludeRegex(CppIncludeRegexPattern);
   SmallVector Matches;
   SmallVector IncludesInBlock;
-
+  
   // In compiled files, consider the first #include to be the main #include of
   // the file if it is not a system #include. This ensures that the header
   // doesn't have hidden dependencies
@@ -2751,7 +2750,19 @@
 bool MergeWithNextLine = Trimmed.endswith("\\");
 if (!FormattingOff && !MergeWithNextLine) {
   if (IncludeRegex.match(Line, &Matches)) {
-StringRef IncludeName = Matches[2];
+StringRef IncludeName;
+for (int i=Matches.size()-1; i>0; i--) {
+  if (!Matches[i].empty()) {
+IncludeName = Matches[i];
+break;
+  }
+}
+// This addresses https://github.com/llvm/llvm-project/issues/38995
+int WithSemicolon = false;
+i

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk updated this revision to Diff 415489.
kwk edited the summary of this revision.
kwk added a comment.

- Fix formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

Files:
  clang/lib/Format/Format.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/unittests/Format/SortIncludesTest.cpp

Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -458,6 +458,39 @@
  "#include \"b.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, SupportAtImportLines) {
+  // Test from https://github.com/llvm/llvm-project/issues/38995
+  EXPECT_EQ("#import \"a.h\"\n"
+"#import \"b.h\"\n"
+"#import \"c.h\"\n"
+"#import \n"
+"@import Foundation;\n",
+sort("#import \"b.h\"\n"
+ "#import \"c.h\"\n"
+ "#import \n"
+ "@import Foundation;\n"
+ "#import \"a.h\"\n"));
+
+  // Slightly more complicated test that shows sorting in each priorities still
+  // works.
+  EXPECT_EQ("#import \"a.h\"\n"
+"#import \"b.h\"\n"
+"#import \"c.h\"\n"
+"#import \n"
+"@import Base;\n"
+"@import Foundation;\n"
+"@import base;\n"
+"@import foundation;\n",
+sort("#import \"b.h\"\n"
+ "#import \"c.h\"\n"
+ "@import Base;\n"
+ "#import \n"
+ "@import foundation;\n"
+ "@import Foundation;\n"
+ "@import base;\n"
+ "#import \"a.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, LeavesMainHeaderFirst) {
   Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
   EXPECT_EQ("#include \"llvm/a.h\"\n"
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -170,11 +170,11 @@
 }
 
 inline StringRef trimInclude(StringRef IncludeName) {
-  return IncludeName.trim("\"<>");
+  return IncludeName.trim("\"<>;");
 }
 
 const char IncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#][\t\ ]*(import|include)([^"]*("[^"]+")|[^<]*(<[^>]+>)|[\t\ ]*([^;]+;)))";
 
 // The filename of Path excluding extension.
 // Used to match implementation with headers, this differs from sys::path::stem:
@@ -290,10 +290,17 @@
   for (auto Line : Lines) {
 NextLineOffset = std::min(Code.size(), Offset + Line.size() + 1);
 if (IncludeRegex.match(Line, &Matches)) {
+  StringRef IncludeName;
+  for (int i = Matches.size() - 1; i > 0; i--) {
+if (!Matches[i].empty()) {
+  IncludeName = Matches[i];
+  break;
+}
+  }
   // If this is the last line without trailing newline, we need to make
   // sure we don't delete across the file boundary.
   addExistingInclude(
-  Include(Matches[2],
+  Include(IncludeName,
   tooling::Range(
   Offset, std::min(Line.size() + 1, Code.size() - Offset))),
   NextLineOffset);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2682,8 +2682,7 @@
 namespace {
 
 const char CppIncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
-
+R"(^[\t\ ]*[@#][\t\ ]*(import|include)([^"]*("[^"]+")|[^<]*(<[^>]+>)|[\t\ ]*([^;]+;)))";
 } // anonymous namespace
 
 tooling::Replacements sortCppIncludes(const FormatStyle &Style, StringRef Code,
@@ -2751,7 +2750,20 @@
 bool MergeWithNextLine = Trimmed.endswith("\\");
 if (!FormattingOff && !MergeWithNextLine) {
   if (IncludeRegex.match(Line, &Matches)) {
-StringRef IncludeName = Matches[2];
+StringRef IncludeName;
+for (int i = Matches.size() - 1; i > 0; i--) {
+  if (!Matches[i].empty()) {
+IncludeName = Matches[i];
+break;
+  }
+}
+// This addresses https://github.com/llvm/llvm-project/issues/38995
+int WithSemicolon = false;
+if (!IncludeName.startswith("\"") && !IncludeName.startswith("<") &&
+IncludeName.endswith(";")) {
+  WithSemicolon = true;
+}
+
 if (Line.contains("/*") && !Line.contains("*/")) {
   // #include with a start of a block comment, but without the end.
   // Need to keep all the lines until the end of the comment together.
@@ -2764,8 +2776,10 @@
 int Category = Categories.getIncludePriority(
 IncludeName,
 /*CheckMainHeader=*/!MainIncludeF

[PATCH] D121715: [Clang] Fix an unused-but-set-variable warning with compond assignment

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/test/Sema/warn-unused-but-set-variables.c:66
+  for (int i = 0; i < 1000; i++)
+j += 1;
+  return;

The handling of this testcase without your patch seems fine.  Even if the value 
of j is technically read, it's not "used" in any meaningful way.

I think the distinguishing factor for the testcase we discussed before is the 
use of a "volatile".  Volatile is rare, and indicates the user is intentionally 
doing something unusual, so it probably doesn't make sense to warn.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121715

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

[clang] ba59476 - Revert "[clang][driver] Emit a warning if -xc/-xc++ is after the last input file"

[Yi Kong via cfe-commits]

Author: Yi Kong
Date: 2022-03-16T01:17:17+08:00
New Revision: ba59476515cf4598dd25bcfacfbca11b4f4da3d4

URL: 
https://github.com/llvm/llvm-project/commit/ba59476515cf4598dd25bcfacfbca11b4f4da3d4
DIFF: 
https://github.com/llvm/llvm-project/commit/ba59476515cf4598dd25bcfacfbca11b4f4da3d4.diff

LOG: Revert "[clang][driver] Emit a warning if -xc/-xc++ is after the last 
input file"

This reverts commit 1c1a4b9556db8579f7428605ac2c351bddde9ad5.

Some builders failed.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/Driver.cpp

Removed: 
clang/test/Driver/x-args.c



diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 3394eefd738c4..afedb37797e32 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -365,9 +365,6 @@ def warn_drv_preprocessed_input_file_unused : Warning<
 def warn_drv_unused_argument : Warning<
   "argument unused during compilation: '%0'">,
   InGroup;
-def warn_drv_unused_x : Warning<
-  "‘-x %0’ after last input file has no effect">,
-  InGroup;
 def warn_drv_empty_joined_argument : Warning<
   "joined argument expects additional value: '%0'">,
   InGroup;

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index d7a5ca4f21988..70822490e63b8 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -2308,15 +2308,6 @@ void Driver::BuildInputs(const ToolChain &TC, 
DerivedArgList &Args,
 assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }
 
-  // Warn -x after last input file has no effect
-  {
-Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
-Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
-if (LastInputArg->getIndex() < LastXArg->getIndex()) {
-  Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
-}
-  }
-
   for (Arg *A : Args) {
 if (A->getOption().getKind() == Option::InputClass) {
   const char *Value = A->getValue();

diff  --git a/clang/test/Driver/x-args.c b/clang/test/Driver/x-args.c
deleted file mode 100644
index 9b095fcd6c7f9..0
--- a/clang/test/Driver/x-args.c
+++ /dev/null
@@ -1,7 +0,0 @@
-// RUN: %clang -fsyntax-only -Werror -xc %s
-// RUN: %clang -fsyntax-only -Werror %s -xc %s
-
-// RUN: %clang -fsyntax-only %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
-// RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
-// CHECK: ‘-x c++’ after last input file has no effect
\ No newline at end of file



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

[PATCH] D121683: Emit a warning if -xc/-xc++ is after the last input file

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/x-args.c:1
+// RUN: %clang -fsyntax-only -Werror -xc %s
+// RUN: %clang -fsyntax-only -Werror %s -xc %s

The first two RUN lines are unneeded



Comment at: clang/test/Driver/x-args.c:8
+// CHECK: ‘-x c++’ after last input file has no effect
\ No newline at end of file


No newline at end of file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121683

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

[PATCH] D121683: Emit a warning if -xc/-xc++ is after the last input file

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:369
+def warn_drv_unused_x : Warning<
+  "‘-x %0’ after last input file has no effect">,
+  InGroup;

Use ASCII punctuation marks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121683

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

[PATCH] D121683: Emit a warning if -xc/-xc++ is after the last input file

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:2316
+if (LastInputArg->getIndex() < LastXArg->getIndex()) {
+  Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+}

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

Don’t Use Braces on Simple Single-Statement Bodies of if/else/loop Statements

Is it guaranteed that both LastXArg and LastInputArg will be non-null?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121683

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

[PATCH] D111587: re-land: [clang] Fix absolute file paths with -fdebug-prefix-map

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D111587#3382834 , @keith wrote:

> I actually mean dwarf 6, which appears to be partially implemented according 
> to https://lists.llvm.org/pipermail/llvm-dev/2020-April/141055.html
>
> I discovered the issue from the failed tests on 
> https://reviews.llvm.org/D113718 where you can see the test output contains a 
> checksum that otherwise doesn't appear. Passing `-dwarf-version=6` reproduces 
> the issue.

That link describes extensions that might or might not become part of dwarf 6 
in the future.  But there is no "dwarf 6" today.  I did a quick grep and don't 
see any places that check for v6.  File checksums are part of dwarf 5, so 
presumably the failure reproduces with `-dwarf-version=5` as well?  That would 
make more sense.

If you really do need `-dwarf-version=6` to reproduce the problem, then 
somebody has done something very wrong.  Sorry to hold up your review but I 
very much want to get this clarified.




Comment at: clang/test/Modules/module-debuginfo-prefix.m:24
 
-// Dir should always be empty, but on Windows we can't recognize /var
-// as being an absolute path.
-// CHECK: !DIFile(filename: "/OVERRIDE/DebugObjC.h", directory: 
"{{()|(.*:.*)}}")
+// CHECK: !DIFile(filename: "{{/|.:}}OVERRIDE{{/|}}DebugObjC.h", 
directory: "")

keith wrote:
> probinson wrote:
> > Does this want to be 
> > `"%{fs-src-root}OVERRIDE%{fs-sep}DebugObjC.h"` ?
> That can only be used in the RUN invocations, but unfortunately regardless it 
> could be used here because of the escaped backslashes, so it requires a regex 
> like this instead 
Ah, got it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111587

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