date:20230730

[PATCH] D155371: [clang][Interp] Implement __builtin_isinf

2023-07-30 Thread Timm Bäder 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 rG72450a77937c: [clang][Interp] Implement __builtin_isinf 
(authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D155371?vs=540693&id=545515#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155371

Files:
  clang/lib/AST/Interp/Floating.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtin-functions.cpp


Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -72,3 +72,8 @@
   constexpr float min3 = __builtin_fmin(__builtin_inf(), __builtin_nan(""));
   static_assert(min3 == __builtin_inf(), "");
 }
+
+namespace inf {
+  static_assert(__builtin_isinf(__builtin_inf()), "");
+  static_assert(!__builtin_isinf(1.0), "");
+}
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -173,6 +173,20 @@
   return true;
 }
 
+static bool interp__builtin_isinf(InterpState &S, CodePtr OpPC,
+  const InterpFrame *Frame, const Function *F,
+  bool CheckSign) {
+  const Floating &Arg = S.Stk.peek();
+  bool IsInf = Arg.isInf();
+
+  if (CheckSign)
+S.Stk.push>(
+Integral<32, true>::from(IsInf ? (Arg.isNegative() ? -1 : 1) : 0));
+  else
+S.Stk.push>(Integral<32, true>::from(Arg.isInf()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -239,6 +253,16 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isinf:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/false))
+  return Ret(S, OpPC, Dummy);
+break;
+
+  case Builtin::BI__builtin_isinf_sign:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/true))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }
Index: clang/lib/AST/Interp/Floating.h
===
--- clang/lib/AST/Interp/Floating.h
+++ clang/lib/AST/Interp/Floating.h
@@ -87,6 +87,7 @@
   bool isMin() const { return F.isSmallest(); }
   bool isMinusOne() const { return F.isExactlyValue(-1.0); }
   bool isNan() const { return F.isNaN(); }
+  bool isInf() const { return F.isInfinity(); }
   bool isFinite() const { return F.isFinite(); }
 
   ComparisonCategoryResult compare(const Floating &RHS) const {


Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -72,3 +72,8 @@
   constexpr float min3 = __builtin_fmin(__builtin_inf(), __builtin_nan(""));
   static_assert(min3 == __builtin_inf(), "");
 }
+
+namespace inf {
+  static_assert(__builtin_isinf(__builtin_inf()), "");
+  static_assert(!__builtin_isinf(1.0), "");
+}
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -173,6 +173,20 @@
   return true;
 }
 
+static bool interp__builtin_isinf(InterpState &S, CodePtr OpPC,
+  const InterpFrame *Frame, const Function *F,
+  bool CheckSign) {
+  const Floating &Arg = S.Stk.peek();
+  bool IsInf = Arg.isInf();
+
+  if (CheckSign)
+S.Stk.push>(
+Integral<32, true>::from(IsInf ? (Arg.isNegative() ? -1 : 1) : 0));
+  else
+S.Stk.push>(Integral<32, true>::from(Arg.isInf()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -239,6 +253,16 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isinf:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/false))
+  return Ret(S, OpPC, Dummy);
+break;
+
+  case Builtin::BI__builtin_isinf_sign:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/true))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }
Index: clang/lib/AST/Interp/Floating.h
===
--- clang/lib/AST/Interp/Floating.h
+++ clang/lib/AST/Interp/Floating.h
@@ -87,6 +87,7 @@
   bool isMin() const { return F.isSmallest(); }
   bool isMinusOne() const { return F.isExactlyValue(-1.0); }
   bool isNan() const { return F.isNaN(); }
+  bool isInf() const { return F.isInfinity(); }
   bool isFinite() const

[clang] 72450a7 - [clang][Interp] Implement __builtin_isinf

2023-07-30 Thread Timm Bäder via cfe-commits


Author: Timm Bäder
Date: 2023-07-31T08:49:22+02:00
New Revision: 72450a77937c3d47f144b20cf16b595955583a0c

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

LOG: [clang][Interp] Implement __builtin_isinf

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

Added: 


Modified: 
clang/lib/AST/Interp/Floating.h
clang/lib/AST/Interp/InterpBuiltin.cpp
clang/test/AST/Interp/builtin-functions.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/Floating.h b/clang/lib/AST/Interp/Floating.h
index 447361a43696b0..f98f49a7afb70e 100644
--- a/clang/lib/AST/Interp/Floating.h
+++ b/clang/lib/AST/Interp/Floating.h
@@ -87,6 +87,7 @@ class Floating final {
   bool isMin() const { return F.isSmallest(); }
   bool isMinusOne() const { return F.isExactlyValue(-1.0); }
   bool isNan() const { return F.isNaN(); }
+  bool isInf() const { return F.isInfinity(); }
   bool isFinite() const { return F.isFinite(); }
 
   ComparisonCategoryResult compare(const Floating &RHS) const {

diff  --git a/clang/lib/AST/Interp/InterpBuiltin.cpp 
b/clang/lib/AST/Interp/InterpBuiltin.cpp
index 01c41339bba8c8..b5e19af5c37288 100644
--- a/clang/lib/AST/Interp/InterpBuiltin.cpp
+++ b/clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -173,6 +173,20 @@ static bool interp__builtin_isnan(InterpState &S, CodePtr 
OpPC,
   return true;
 }
 
+static bool interp__builtin_isinf(InterpState &S, CodePtr OpPC,
+  const InterpFrame *Frame, const Function *F,
+  bool CheckSign) {
+  const Floating &Arg = S.Stk.peek();
+  bool IsInf = Arg.isInf();
+
+  if (CheckSign)
+S.Stk.push>(
+Integral<32, true>::from(IsInf ? (Arg.isNegative() ? -1 : 1) : 0));
+  else
+S.Stk.push>(Integral<32, true>::from(Arg.isInf()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -239,6 +253,16 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const 
Function *F) {
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isinf:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/false))
+  return Ret(S, OpPC, Dummy);
+break;
+
+  case Builtin::BI__builtin_isinf_sign:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/true))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }

diff  --git a/clang/test/AST/Interp/builtin-functions.cpp 
b/clang/test/AST/Interp/builtin-functions.cpp
index 8ff717217e4728..7eb3e187d28ead 100644
--- a/clang/test/AST/Interp/builtin-functions.cpp
+++ b/clang/test/AST/Interp/builtin-functions.cpp
@@ -72,3 +72,8 @@ namespace fmin {
   constexpr float min3 = __builtin_fmin(__builtin_inf(), __builtin_nan(""));
   static_assert(min3 == __builtin_inf(), "");
 }
+
+namespace inf {
+  static_assert(__builtin_isinf(__builtin_inf()), "");
+  static_assert(!__builtin_isinf(1.0), "");
+}



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

[clang-tools-extra] 575900d - [clang-tidy] Add bugprone-optional-value-conversion check

2023-07-30 Thread Piotr Zegar via cfe-commits


Author: Piotr Zegar
Date: 2023-07-31T06:22:39Z
New Revision: 575900d0d98aafce9d534b6c1917861311f3cfaa

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

LOG: [clang-tidy] Add bugprone-optional-value-conversion check

Detects potentially unintentional and redundant conversions where a value is
extracted from an optional-like type and then used to create a new instance of
the same optional-like type.

Reviewed By: xgupta

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

Added: 
clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.h

clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst

clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp

Modified: 
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 853c6d51458793..a52bb91df0ca8f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -44,6 +44,7 @@
 #include "NoEscapeCheck.h"
 #include "NonZeroEnumToBoolConversionCheck.h"
 #include "NotNullTerminatedResultCheck.h"
+#include "OptionalValueConversionCheck.h"
 #include "ParentVirtualCallCheck.h"
 #include "PosixReturnCheck.h"
 #include "RedundantBranchConditionCheck.h"
@@ -150,6 +151,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-multiple-new-in-one-expression");
 CheckFactories.registerCheck(
 "bugprone-multiple-statement-macro");
+CheckFactories.registerCheck(
+"bugprone-optional-value-conversion");
 CheckFactories.registerCheck(
 "bugprone-redundant-branch-condition");
 CheckFactories.registerCheck(

diff  --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 5669d6bf4205f6..b329857785b067 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -40,6 +40,7 @@ add_clang_library(clangTidyBugproneModule
   NoEscapeCheck.cpp
   NonZeroEnumToBoolConversionCheck.cpp
   NotNullTerminatedResultCheck.cpp
+  OptionalValueConversionCheck.cpp
   ParentVirtualCallCheck.cpp
   PosixReturnCheck.cpp
   RedundantBranchConditionCheck.cpp

diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
new file mode 100644
index 00..9ab59e6b0474f0
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
@@ -0,0 +1,126 @@
+//===--- OptionalValueConversionCheck.cpp - clang-tidy 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "OptionalValueConversionCheck.h"
+#include "../utils/LexerUtils.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(QualType, hasCleanType, 
ast_matchers::internal::Matcher,
+  InnerMatcher) {
+  return InnerMatcher.matches(
+  Node.getNonReferenceType().getUnqualifiedType().getCanonicalType(),
+  Finder, Builder);
+}
+
+} // namespace
+
+OptionalValueConversionCheck::OptionalValueConversionCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  OptionalTypes(utils::options::parseStringList(
+  Options.get("OptionalTypes",
+  "::std::optional;::absl::optional;::boost::optional"))),
+  ValueMethods(utils::options::parseStringList(
+  Options.get("ValueMethods", "::value$;::get$"))) {}
+
+std::optional
+OptionalValueConversionCheck::getCheckTraversalKind() const {
+  return TK_AsIs;
+}
+
+void OptionalValueConversionCheck::registerMatchers(MatchFinder *Finder) {
+  auto ConstructTypeMatcher =
+  qualType(hasCleanType(qualType().bind("optional-type")));
+
+  auto CallTypeMatcher =
+  qualType(hasCleanType(equalsBoundNode("optional-type")));
+
+  auto OptionalDereferenceMatcher = cal

[PATCH] D147357: [clang-tidy] Add bugprone-optional-value-conversion check

2023-07-30 Thread Piotr Zegar via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG575900d0d98a: [clang-tidy] Add 
bugprone-optional-value-conversion check (authored by PiotrZSL).

Changed prior to commit:
  https://reviews.llvm.org/D147357?vs=545464&id=545510#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
@@ -0,0 +1,213 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-optional-value-conversion %t -- --fix-notes
+// RUN: %check_clang_tidy -check-suffix=CUSTOM -std=c++17-or-later %s bugprone-optional-value-conversion %t -- \
+// RUN: -config="{CheckOptions: [{key: 'bugprone-optional-value-conversion.OptionalTypes', value: 'CustomOptional'}, \
+// RUN:  {key: 'bugprone-optional-value-conversion.ValueMethods', value: '::Read$;::Ooo$'}]}" --fix-notes
+
+namespace std {
+  template
+  struct optional
+  {
+constexpr optional() noexcept;
+constexpr optional(T&&) noexcept;
+constexpr optional(const T&) noexcept;
+template
+constexpr optional(U&&) noexcept;
+const T& operator*() const;
+T* operator->();
+const T* operator->() const;
+T& operator*();
+const T& value() const;
+T& value();
+const T& get() const;
+T& get();
+T value_or(T) const;
+  };
+
+  template 
+  T&& move(T &x) {
+return static_cast(x);
+  }
+}
+
+namespace boost {
+  template
+  struct optional {
+constexpr optional() noexcept;
+constexpr optional(const T&) noexcept;
+const T& operator*() const;
+const T& get() const;
+  };
+}
+
+namespace absl {
+  template
+  struct optional {
+constexpr optional() noexcept;
+constexpr optional(const T&) noexcept;
+const T& operator*() const;
+const T& value() const;
+  };
+}
+
+template
+struct CustomOptional {
+  CustomOptional();
+  CustomOptional(const T&);
+  const T& Read() const;
+  T& operator*();
+  T& Ooo();
+};
+
+void takeOptionalValue(std::optional);
+void takeOptionalRef(const std::optional&);
+void takeOptionalRRef(std::optional&&);
+void takeOtherOptional(std::optional);
+void takeBOptionalValue(boost::optional);
+void takeAOptionalValue(absl::optional);
+
+void incorrect(std::optional param)
+{
+  std::optional* ptr = ¶m;
+  takeOptionalValue(**ptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalValue(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(param.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(ptr->value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalValue(param.operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(ptr->operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalRef(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remov

[PATCH] D156274: [NFC] Avoid potential dereferencing of nullptr.

2023-07-30 Thread Sindhu Chittireddy via Phabricator via cfe-commits

schittir added a comment.

In D156274#4544804 , @aaron.ballman 
wrote:

> LGTM! Are you planning to work on the ObjC issues separately? (Not suggesting 
> you have to! But if you don't intend to, can you file an issue in GitHub so 
> we don't lose track of the problem?)

Hi Aaron, 
I will make sure to file a bug report. 
Thank you!


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

https://reviews.llvm.org/D156274

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

[PATCH] D156274: [NFC] Avoid potential dereferencing of nullptr.

2023-07-30 Thread Sindhu Chittireddy via Phabricator via cfe-commits

schittir added a comment.

Test failure seems unrelated to change. Driver/fsanitize.c lit test passes in 
my local testing. Restarting build.


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

https://reviews.llvm.org/D156274

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

[PATCH] D156515: [RemarkUtil] Refactor llvm-remarkutil to include size-diff

2023-07-30 Thread Jessica Paquette via Phabricator via cfe-commits

paquette accepted this revision.
paquette added a comment.

LGTM too thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156515

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

[PATCH] D156643: [clang-format][NFC] Remove superfluous code in UnwrappedLineParser

2023-07-30 Thread Owen Pan via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156643

Files:
  clang/lib/Format/UnwrappedLineParser.cpp


Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1188,8 +1188,8 @@
 static bool tokenCanStartNewLine(const FormatToken &Tok) {
   // Semicolon can be a null-statement, l_square can be a start of a macro or
   // a C++11 attribute, but this doesn't seem to be common.
+  assert(Tok.isNot(TT_AttributeSquare));
   return Tok.isNot(tok::semi) && Tok.isNot(tok::l_brace) &&
- Tok.isNot(TT_AttributeSquare) &&
  // Tokens that can only be used as binary operators and a part of
  // overloaded operator names.
  Tok.isNot(tok::period) && Tok.isNot(tok::periodstar) &&
@@ -3646,12 +3646,7 @@
 // We can have macros or attributes in between 'enum' and the enum name.
 if (FormatTok->is(tok::l_paren))
   parseParens();
-if (FormatTok->is(TT_AttributeSquare)) {
-  parseSquare();
-  // Consume the closing TT_AttributeSquare.
-  if (FormatTok->Next && FormatTok->is(TT_AttributeSquare))
-nextToken();
-}
+assert(FormatTok->isNot(TT_AttributeSquare));
 if (FormatTok->is(tok::identifier)) {
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate


Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1188,8 +1188,8 @@
 static bool tokenCanStartNewLine(const FormatToken &Tok) {
   // Semicolon can be a null-statement, l_square can be a start of a macro or
   // a C++11 attribute, but this doesn't seem to be common.
+  assert(Tok.isNot(TT_AttributeSquare));
   return Tok.isNot(tok::semi) && Tok.isNot(tok::l_brace) &&
- Tok.isNot(TT_AttributeSquare) &&
  // Tokens that can only be used as binary operators and a part of
  // overloaded operator names.
  Tok.isNot(tok::period) && Tok.isNot(tok::periodstar) &&
@@ -3646,12 +3646,7 @@
 // We can have macros or attributes in between 'enum' and the enum name.
 if (FormatTok->is(tok::l_paren))
   parseParens();
-if (FormatTok->is(TT_AttributeSquare)) {
-  parseSquare();
-  // Consume the closing TT_AttributeSquare.
-  if (FormatTok->Next && FormatTok->is(TT_AttributeSquare))
-nextToken();
-}
+assert(FormatTok->isNot(TT_AttributeSquare));
 if (FormatTok->is(tok::identifier)) {
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c652525 - [CodeGen] Remove unused declaration/function BlockCaptureManagedEntity

2023-07-30 Thread Kazu Hirata via cfe-commits


Author: Kazu Hirata
Date: 2023-07-30T21:22:26-07:00
New Revision: c652525295f6b4eac0f4a38f16ca565b1187d17d

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

LOG: [CodeGen] Remove unused declaration/function BlockCaptureManagedEntity

The last use was removed by:

  commit e5df9cc098b554ebb066792e40cbde6feddc57bc
  Author: Akira Hatanaka 
  Date:   Sat Jan 8 13:27:28 2022 -0800

Added: 


Modified: 
clang/lib/CodeGen/CGBlocks.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index cfbe3272196e35..2023be8838f8a7 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -66,28 +66,6 @@ static llvm::Constant *buildDisposeHelper(CodeGenModule &CGM,
 
 namespace {
 
-/// Represents a captured entity that requires extra operations in order for
-/// this entity to be copied or destroyed correctly.
-struct BlockCaptureManagedEntity {
-  BlockCaptureEntityKind CopyKind, DisposeKind;
-  BlockFieldFlags CopyFlags, DisposeFlags;
-  const BlockDecl::Capture *CI;
-  const CGBlockInfo::Capture *Capture;
-
-  BlockCaptureManagedEntity(BlockCaptureEntityKind CopyType,
-BlockCaptureEntityKind DisposeType,
-BlockFieldFlags CopyFlags,
-BlockFieldFlags DisposeFlags,
-const BlockDecl::Capture &CI,
-const CGBlockInfo::Capture &Capture)
-  : CopyKind(CopyType), DisposeKind(DisposeType), CopyFlags(CopyFlags),
-DisposeFlags(DisposeFlags), CI(&CI), Capture(&Capture) {}
-
-  bool operator<(const BlockCaptureManagedEntity &Other) const {
-return Capture->getOffset() < Other.Capture->getOffset();
-  }
-};
-
 enum class CaptureStrKind {
   // String for the copy helper.
   CopyHelper,



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

[PATCH] D156210: [ODRHash] Hash type-as-written

2023-07-30 Thread Chuanqi Xu 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 rGc31d6b4ef135: [ODRHash] Hash type-as-written (authored by 
ChuanqiXu).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156210

Files:
  clang/lib/AST/ODRHash.cpp
  clang/test/Modules/odr_hash-gnu.cpp
  clang/test/Modules/odr_hash.cpp
  clang/test/Modules/odr_hash.mm
  clang/test/Modules/pr63595.cppm

Index: clang/test/Modules/pr63595.cppm
===
--- /dev/null
+++ clang/test/Modules/pr63595.cppm
@@ -0,0 +1,56 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/module1.cppm -o %t/module1.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/module2.cppm -o %t/module2.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/merge.cpp -verify -fsyntax-only
+
+//--- header.h
+namespace NS {
+template 
+class A {
+};
+
+template  class T>
+class B {
+};
+}
+
+//--- module1.h
+namespace NS {
+using C = B;
+}
+struct D : NS::C {
+using Type = NS::C;
+};
+
+//--- module1.cppm
+// inside NS, using C = B
+module;
+#include "header.h"
+#include "module1.h"
+export module module1;
+export using ::D;
+
+//--- module2.h
+namespace NS {
+using C = B;
+}
+struct D : NS::C {
+using Type = NS::C;
+};
+
+//--- module2.cppm
+// inside NS, using C = B
+module;
+#include "header.h"
+#include "module2.h"
+export module module2;
+export using ::D;
+
+//--- merge.cpp
+// expected-no-diagnostics
+import module1;
+import module2;
+D d;
Index: clang/test/Modules/odr_hash.mm
===
--- clang/test/Modules/odr_hash.mm
+++ clang/test/Modules/odr_hash.mm
@@ -112,8 +112,7 @@
 // expected-error@second.h:* {{Types::Attributed::invalid1' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
 // expected-note@first.h:* {{but in 'FirstModule' found a different body}}
 auto function2 = invalid2;
-// expected-error@second.h:* {{'Types::Attributed::invalid2' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
-// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
+
 auto function3 = valid;
 #endif
 }  // namespace Attributed
@@ -266,12 +265,7 @@
 }
 @end
 #else
-// expected-error@first.h:* {{'Interface4::x' from module 'FirstModule' is not present in definition of 'Interface4' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'x' does not match}}
-// expected-error@first.h:* {{'Interface5::y' from module 'FirstModule' is not present in definition of 'Interface5' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'y' does not match}}
-// expected-error@first.h:* {{'Interface6::z' from module 'FirstModule' is not present in definition of 'Interface6' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'z' does not match}}
+
 #endif
 
 namespace Types {
@@ -291,14 +285,14 @@
 };
 #else
 Invalid1 i1;
-// expected-error@first.h:* {{'Types::ObjCTypeParam::Invalid1::x' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid1' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'x' does not match}}
+
 Invalid2 i2;
-// expected-error@first.h:* {{'Types::ObjCTypeParam::Invalid2::y' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid2' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'y' does not match}}
+
 Invalid3 i3;
-// expected-error@first.h:* {{'Types::ObjCTypeParam::Invalid3::z' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid3' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'z' does not match}}
+
+// FIXME: We should reject to merge these structs and diagnose for the
+// different definitions for Interface4/Interface5/Interface6.
+
 #endif
 
 }  // namespace ObjCTypeParam
Index: clang/test/Modules/odr_hash.cpp
===
--- clang/test/Modules/odr_hash.cpp
+++ clang/test/Modules/odr_hash.cpp
@@ -1046,8 +1046,7 @@
 };
 #else
 S3 s3;
-// expected-error@first.h:* {{'TypeDef::S3::a' from module 'FirstModule' is not present in definition of 'TypeDef::S3' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'a' does not match}}
+// FIXME: We should reject the merge of `S3` due to the inconsistent definition of `T`.
 #endif
 
 #if defined(FIRST)
@@ -1172,8 +1171,7 @@
 };
 #else
 S3 s3;
-// expected-error@first.h:* {{'Using::S3::a' from module 'FirstModule' is not p

[clang] c31d6b4 - [ODRHash] Hash type-as-written

2023-07-30 Thread Chuanqi Xu via cfe-commits


Author: Chuanqi Xu
Date: 2023-07-31T11:05:47+08:00
New Revision: c31d6b4ef135098280b0ebb93e95b258a0d372ca

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

LOG: [ODRHash] Hash type-as-written

Close https://github.com/llvm/llvm-project/issues/63947
Close https://github.com/llvm/llvm-project/issues/63595

This is suggested by @rsmith in
https://reviews.llvm.org/D154324#inline-1508868

Reviewed By: rsmith

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

Added: 
clang/test/Modules/pr63595.cppm

Modified: 
clang/lib/AST/ODRHash.cpp
clang/test/Modules/odr_hash-gnu.cpp
clang/test/Modules/odr_hash.cpp
clang/test/Modules/odr_hash.mm

Removed: 




diff  --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp
index 507fb0b49f8ad3..5eab315793b411 100644
--- a/clang/lib/AST/ODRHash.cpp
+++ b/clang/lib/AST/ODRHash.cpp
@@ -295,9 +295,9 @@ class ODRDeclVisitor : public 
ConstDeclVisitor {
   }
 
   void VisitValueDecl(const ValueDecl *D) {
-if (!isa(D)) {
-  AddQualType(D->getType());
-}
+if (auto *DD = dyn_cast(D); DD && DD->getTypeSourceInfo())
+  AddQualType(DD->getTypeSourceInfo()->getType());
+
 Inherited::VisitValueDecl(D);
   }
 
@@ -351,7 +351,7 @@ class ODRDeclVisitor : public 
ConstDeclVisitor {
   void VisitObjCPropertyDecl(const ObjCPropertyDecl *D) {
 ID.AddInteger(D->getPropertyAttributes());
 ID.AddInteger(D->getPropertyImplementation());
-AddQualType(D->getType());
+AddQualType(D->getTypeSourceInfo()->getType());
 AddDecl(D);
 
 Inherited::VisitObjCPropertyDecl(D);
@@ -394,7 +394,9 @@ class ODRDeclVisitor : public 
ConstDeclVisitor {
 
 AddDecl(Method);
 
-AddQualType(Method->getReturnType());
+if (Method->getReturnTypeSourceInfo())
+  AddQualType(Method->getReturnTypeSourceInfo()->getType());
+
 ID.AddInteger(Method->param_size());
 for (auto Param : Method->parameters())
   Hash.AddSubDecl(Param);
@@ -593,7 +595,7 @@ void ODRHash::AddCXXRecordDecl(const CXXRecordDecl *Record) 
{
   ID.AddInteger(Record->getNumBases());
   auto Bases = Record->bases();
   for (const auto &Base : Bases) {
-AddQualType(Base.getType());
+AddQualType(Base.getTypeSourceInfo()->getType());
 ID.AddInteger(Base.isVirtual());
 ID.AddInteger(Base.getAccessSpecifierAsWritten());
   }
@@ -912,29 +914,7 @@ class ODRTypeVisitor : public TypeVisitor {
   void VisitType(const Type *T) {}
 
   void VisitAdjustedType(const AdjustedType *T) {
-QualType Original = T->getOriginalType();
-QualType Adjusted = T->getAdjustedType();
-
-// The original type and pointee type can be the same, as in the case of
-// function pointers decaying to themselves.  Set a bool and only process
-// the type once, to prevent doubling the work.
-SplitQualType split = Adjusted.split();
-if (auto Pointer = dyn_cast(split.Ty)) {
-  if (Pointer->getPointeeType() == Original) {
-Hash.AddBoolean(true);
-ID.AddInteger(split.Quals.getAsOpaqueValue());
-AddQualType(Original);
-VisitType(T);
-return;
-  }
-}
-
-// The original type and pointee type are 
diff erent, such as in the case
-// of a array decaying to an element pointer.  Set a bool to false and
-// process both types.
-Hash.AddBoolean(false);
-AddQualType(Original);
-AddQualType(Adjusted);
+AddQualType(T->getOriginalType());
 
 VisitType(T);
   }
@@ -973,7 +953,6 @@ class ODRTypeVisitor : public TypeVisitor {
   void VisitAttributedType(const AttributedType *T) {
 ID.AddInteger(T->getAttrKind());
 AddQualType(T->getModifiedType());
-AddQualType(T->getEquivalentType());
 
 VisitType(T);
   }
@@ -995,7 +974,6 @@ class ODRTypeVisitor : public TypeVisitor {
 
   void VisitDecltypeType(const DecltypeType *T) {
 AddStmt(T->getUnderlyingExpr());
-AddQualType(T->getUnderlyingType());
 VisitType(T);
   }
 
@@ -1184,31 +1162,12 @@ class ODRTypeVisitor : public 
TypeVisitor {
 
   void VisitTypedefType(const TypedefType *T) {
 AddDecl(T->getDecl());
-QualType UnderlyingType = T->getDecl()->getUnderlyingType();
-VisitQualifiers(UnderlyingType.getQualifiers());
-while (true) {
-  if (const TypedefType *Underlying =
-  dyn_cast(UnderlyingType.getTypePtr())) {
-UnderlyingType = Underlying->getDecl()->getUnderlyingType();
-continue;
-  }
-  if (const ElaboratedType *Underlying =
-  dyn_cast(UnderlyingType.getTypePtr())) {
-UnderlyingType = Underlying->getNamedType();
-continue;
-  }
-
-  break;
-}
-AddType(UnderlyingType.getTypePtr());
 VisitType(T);
   }
 
   void VisitTypeOfExprType(const TypeOfExprType *T) {
 AddStmt(T->getUnderlying

[PATCH] D156641: [Clang][Frontend] Change help text for --offload-host-device

2023-07-30 Thread Anton Rydahl via Phabricator via cfe-commits

AntonRydahl created this revision.
AntonRydahl added a reviewer: jdoerfert.
Herald added a reviewer: awarzynski.
Herald added projects: Flang, All.
AntonRydahl requested review of this revision.
Herald added subscribers: cfe-commits, wangpc.
Herald added a project: clang.

I believe the help text for the flag `--offload-host-device` is wrong. 
Currently, the help text says "Only compile for the offloading host." but the 
flag alias with `--cuda-compile-host-device` which 
has the help text "Compile CUDA code for both host and device (default). Has no 
effect on non-CUDA compilations."


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156641

Files:
  clang/include/clang/Driver/Options.td
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90


Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -80,7 +80,7 @@
 ! HELP-NEXT: -module-dir   Put MODULE files in 
 ! HELP-NEXT: -nocpp Disable predefined and command line 
preprocessor macros
 ! HELP-NEXT: --offload-device-only   Only compile for the offloading device.
-! HELP-NEXT: --offload-host-device   Only compile for the offloading host.
+! HELP-NEXT: --offload-host-device   Compile for both the offloading host and 
device (default).
 ! HELP-NEXT: --offload-host-only Only compile for the offloading host.
 ! HELP-NEXT: -o   Write output to 
 ! HELP-NEXT: -pedantic  Warn on language extensions
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -84,7 +84,7 @@
 ! CHECK-NEXT: -module-dir   Put MODULE files in 
 ! CHECK-NEXT: -nocpp Disable predefined and command line 
preprocessor macros
 ! CHECK-NEXT: --offload-device-only   Only compile for the offloading device.
-! CHECK-NEXT: --offload-host-device   Only compile for the offloading host.
+! CHECK-NEXT: --offload-host-device   Compile for both the offloading host and 
device (default).
 ! CHECK-NEXT: --offload-host-only Only compile for the offloading host.
 ! CHECK-NEXT: -o  Write output to 
 ! CHECK-NEXT: -pedantic  Warn on language extensions
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2875,7 +2875,7 @@
 def offload_host_only : Flag<["--"], "offload-host-only">, 
Flags<[FlangOption]>,
   HelpText<"Only compile for the offloading host.">;
 def offload_host_device : Flag<["--"], "offload-host-device">, 
Flags<[FlangOption]>,
-  HelpText<"Only compile for the offloading host.">;
+  HelpText<"Compile for both the offloading host and device (default).">;
 def cuda_device_only : Flag<["--"], "cuda-device-only">, 
Alias,
   HelpText<"Compile CUDA code for device only">;
 def cuda_host_only : Flag<["--"], "cuda-host-only">, Alias,


Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -80,7 +80,7 @@
 ! HELP-NEXT: -module-dir   Put MODULE files in 
 ! HELP-NEXT: -nocpp Disable predefined and command line preprocessor macros
 ! HELP-NEXT: --offload-device-only   Only compile for the offloading device.
-! HELP-NEXT: --offload-host-device   Only compile for the offloading host.
+! HELP-NEXT: --offload-host-device   Compile for both the offloading host and device (default).
 ! HELP-NEXT: --offload-host-only Only compile for the offloading host.
 ! HELP-NEXT: -o   Write output to 
 ! HELP-NEXT: -pedantic  Warn on language extensions
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -84,7 +84,7 @@
 ! CHECK-NEXT: -module-dir   Put MODULE files in 
 ! CHECK-NEXT: -nocpp Disable predefined and command line preprocessor macros
 ! CHECK-NEXT: --offload-device-only   Only compile for the offloading device.
-! CHECK-NEXT: --offload-host-device   Only compile for the offloading host.
+! CHECK-NEXT: --offload-host-device   Compile for both the offloading host and device (default).
 ! CHECK-NEXT: --offload-host-only Only compile for the offloading host.
 ! CHECK-NEXT: -o  Write output to 
 ! CHECK-NEXT: -pedantic  Warn on language extensions
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2875,7 +2875,7 @@
 def offload_host_only : Flag<["--"], "off

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

2023-07-30 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D156363#4545215 , @dyung wrote:

> If you cannot reproduce this issue, I can take my buildbot offline and try to 
> reproduce it to pinpoint which command is causing the problem, but I won’t be 
> able to do so until Monday.

Thank you, knowing which command caused the failure will be very helpful.
`clang/test/Driver/fsanitize.c` has the most mysterious failure I have dealt 
with for this patch.

FWIW I've checked that the test passes even if `/usr` does not exist (emulated 
by proot) => there is as if no host GCC installation that could affect Driver's 
behavior

  % cat /tmp/Rel/bin/xxx
  #!/bin/zsh
  d=$(dirname "$0")
  exec proot -R /tmp -b /bin -b /lib64 -b /lib $d/clang 
--target=aarch64-linux-musl "$@"
  % cd clang/test/Driver
  % CLANG=/tmp/Rel/bin/xxx /tmp/Rel/bin/llvm-lit -vv fsanitize.c
  .. passed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156363

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

[PATCH] D146054: [RISCV] Add --print-supported-extensions support

2023-07-30 Thread Brandon Wu via Phabricator via cfe-commits

4vtomat updated this revision to Diff 545500.
4vtomat added a comment.

Updated test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/print-supported-extensions.c
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/RISCVISAInfo.cpp

Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -208,6 +208,29 @@
 #endif
 }
 
+void llvm::riscvExtensionsHelp() {
+  outs() << "All available -march extensions for RISC-V\n\n";
+  outs() << '\t' << left_justify("Name", 20) << "Version\n";
+
+  RISCVISAInfo::OrderedExtensionMap ExtMap;
+  for (const auto &E : SupportedExtensions)
+ExtMap[E.Name] = { E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+outs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  outs() << "\nExperimental extensions\n";
+  ExtMap.clear();
+  for (const auto &E : SupportedExperimentalExtensions)
+ExtMap[E.Name] = { E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+outs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  outs() << "\nUse -march to specify the target's extension.\n"
+"For example, clang -march=rv32i_v1p0\n";
+}
+
 static bool stripExperimentalPrefix(StringRef &Ext) {
   return Ext.consume_front("experimental-");
 }
Index: llvm/include/llvm/Support/RISCVISAInfo.h
===
--- llvm/include/llvm/Support/RISCVISAInfo.h
+++ llvm/include/llvm/Support/RISCVISAInfo.h
@@ -22,6 +22,8 @@
   unsigned MinorVersion;
 };
 
+void riscvExtensionsHelp();
+
 class RISCVISAInfo {
 public:
   RISCVISAInfo(const RISCVISAInfo &) = delete;
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/RISCVISAInfo.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -221,6 +222,10 @@
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
 
+  // --print-supported-extensions takes priority over the actual compilation.
+  if (Clang->getFrontendOpts().PrintSupportedExtensions)
+return llvm::riscvExtensionsHelp(), 0;
+
   // Infer the builtin include path if unspecified.
   if (Clang->getHeaderSearchOpts().UseBuiltinIncludes &&
   Clang->getHeaderSearchOpts().ResourceDir.empty())
Index: clang/test/Driver/print-supported-extensions.c
===
--- /dev/null
+++ clang/test/Driver/print-supported-extensions.c
@@ -0,0 +1,129 @@
+/// Test that --print-supported-extensions lists supported extensions.
+
+// REQUIRES: riscv-registered-target
+
+// RUN: %clang --target=riscv64 --print-supported-extensions 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=warning: --strict-whitespace --match-full-lines %s
+
+//   CHECK:Target: riscv64
+//   CHECK:All available -march extensions for RISC-V
+//   CHECK:	NameVersion
+//  CHECK-NEXT:	i   2.1
+//  CHECK-NEXT:	e   2.0
+//  CHECK-NEXT:	m   2.0
+//  CHECK-NEXT:	a   2.1
+//  CHECK-NEXT:	f   2.2
+//  CHECK-NEXT:	d   2.2
+//  CHECK-NEXT:	c   2.0
+//  CHECK-NEXT:	v   1.0
+//  CHECK-NEXT:	h   1.0
+//  CHECK-NEXT:	zicbom  1.0
+//  CHECK-NEXT:	zicbop  1.0
+//  CHECK-NEXT:	zicboz  1.0
+//  CHECK-NEXT:	zicntr  1.0
+//  CHECK-NEXT:	zicsr   2.0
+//  CHECK-NEXT:	zifencei2.0
+//  CHECK-NEXT:	zihintpause 2.0
+//  CHECK-NEXT:	zihpm   1.0
+//  CHECK-NEXT:	zmmul   1.0
+//  CHECK-NEXT:	zawrs   1.0
+//  CHECK-NEXT:	zfh 1.0
+//  CHECK-NEXT:	zfhmin  1.0
+//  CHECK-NEXT:	zfinx   1.0
+//  CHECK-NEXT:	zdinx   1.0
+//  CHECK-NEXT:	zca 1.0
+//  CHECK-NEXT:	zcb 1.0
+//  CHECK-NEXT:	zcd 1.0
+//  CHECK-NEXT:	zce 1.0
+//  CHECK-NEXT:	zcf 1.0
+//  CHECK-NEXT:	zcmp1.0
+//  CHECK-NEXT:	zcmt1.0
+//  CHECK-NEXT:	zba 1.0
+//  CHEC

[PATCH] D146054: [RISCV] Add --print-supported-extensions support

2023-07-30 Thread Brandon Wu via Phabricator via cfe-commits

4vtomat added inline comments.



Comment at: clang/include/clang/Frontend/FrontendOptions.h:290
+  /// Output time trace profile.
+  unsigned TimeTrace : 1;
+

MaskRay wrote:
> stray change?
Oh, maybe it was added accidentally during rebase lol~



Comment at: clang/test/Driver/print-supported-extensions.c:10
+// CHECK:All available -march extensions for RISC-V
+// CHECK:  NameVersion
+// CHECK-NEXT: i   2.1

MaskRay wrote:
> For `--strict-whitespace --match-full-lines` testing, we usually right align 
> `CHECK` and `CHECK-NEXT`.
Oh, I got it, thanks!



Comment at: clang/tools/driver/cc1_main.cpp:187
+/// Print supported extensions of the RISCV target.
+static void printSupportedExtensions() {
+  llvm::riscvExtensionsHelp();

MaskRay wrote:
> The call site should just call riscvExtensionsHelp so that this internal 
> linkage function can be avoided.
Yeah, good idea!



Comment at: clang/tools/driver/cc1_main.cpp:187
+/// Print supported extensions of the RISCV target.
+static int print_supported_extensions() {
+  llvm::riscvExtensionsHelp();

MaskRay wrote:
> `printSupportedExtensions`. Why does this function return a dummy return 
> value?
> 
> You can do `return printSupportedExtensions(), 0` below to save some lines.
This is a good idea, return in its caller is much clear for it's intension!



Comment at: llvm/lib/Support/RISCVISAInfo.cpp:202
+void llvm::riscvExtensionsHelp() {
+  errs() << "All available -march extensions for RISC-V\n\n";
+  errs() << '\t' << left_justify("Name", 20) << "Version\n";

MaskRay wrote:
> I think `outs()` is more conventional. Most `gcc --print-*` options go to 
> stdout. `clang --print-supported-cpus` deviates and we should not copy its 
> issue.
Sure, I agree to keep it conventional, since I was following 
`--print-supported-cpus` so I made it this way, thank you for pointing out this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

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

[PATCH] D146054: [RISCV] Add --print-supported-extensions support

2023-07-30 Thread Brandon Wu via Phabricator via cfe-commits

4vtomat updated this revision to Diff 545499.
4vtomat marked 6 inline comments as done.
4vtomat added a comment.

Resolved MaskRay's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/print-supported-extensions.c
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/RISCVISAInfo.cpp

Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -208,6 +208,29 @@
 #endif
 }
 
+void llvm::riscvExtensionsHelp() {
+  outs() << "All available -march extensions for RISC-V\n\n";
+  outs() << '\t' << left_justify("Name", 20) << "Version\n";
+
+  RISCVISAInfo::OrderedExtensionMap ExtMap;
+  for (const auto &E : SupportedExtensions)
+ExtMap[E.Name] = { E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+outs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  outs() << "\nExperimental extensions\n";
+  ExtMap.clear();
+  for (const auto &E : SupportedExperimentalExtensions)
+ExtMap[E.Name] = { E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+outs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  outs() << "\nUse -march to specify the target's extension.\n"
+"For example, clang -march=rv32i_v1p0\n";
+}
+
 static bool stripExperimentalPrefix(StringRef &Ext) {
   return Ext.consume_front("experimental-");
 }
Index: llvm/include/llvm/Support/RISCVISAInfo.h
===
--- llvm/include/llvm/Support/RISCVISAInfo.h
+++ llvm/include/llvm/Support/RISCVISAInfo.h
@@ -22,6 +22,8 @@
   unsigned MinorVersion;
 };
 
+void riscvExtensionsHelp();
+
 class RISCVISAInfo {
 public:
   RISCVISAInfo(const RISCVISAInfo &) = delete;
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/RISCVISAInfo.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -221,6 +222,10 @@
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
 
+  // --print-supported-extensions takes priority over the actual compilation.
+  if (Clang->getFrontendOpts().PrintSupportedExtensions)
+return llvm::riscvExtensionsHelp(), 0;
+
   // Infer the builtin include path if unspecified.
   if (Clang->getHeaderSearchOpts().UseBuiltinIncludes &&
   Clang->getHeaderSearchOpts().ResourceDir.empty())
Index: clang/test/Driver/print-supported-extensions.c
===
--- /dev/null
+++ clang/test/Driver/print-supported-extensions.c
@@ -0,0 +1,122 @@
+/// Test that --print-supported-extensions lists supported extensions.
+
+// REQUIRES: riscv-registered-target
+
+// RUN: %clang --target=riscv64 --print-supported-extensions 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=warning: --strict-whitespace --match-full-lines %s
+
+//  CHECK:Target: riscv64
+//  CHECK:All available -march extensions for RISC-V
+//  CHECK:	NameVersion
+// CHECK-NEXT:	i   2.1
+// CHECK-NEXT:	e   2.0
+// CHECK-NEXT:	m   2.0
+// CHECK-NEXT:	a   2.1
+// CHECK-NEXT:	f   2.2
+// CHECK-NEXT:	d   2.2
+// CHECK-NEXT:	c   2.0
+// CHECK-NEXT:	v   1.0
+// CHECK-NEXT:	h   1.0
+// CHECK-NEXT:	zicbom  1.0
+// CHECK-NEXT:	zicbop  1.0
+// CHECK-NEXT:	zicboz  1.0
+// CHECK-NEXT:	zicntr  1.0
+// CHECK-NEXT:	zicsr   2.0
+// CHECK-NEXT:	zifencei2.0
+// CHECK-NEXT:	zihintpause 2.0
+// CHECK-NEXT:	zihpm   1.0
+// CHECK-NEXT:	zmmul   1.0
+// CHECK-NEXT:	zawrs   1.0
+// CHECK-NEXT:	zfh 1.0
+// CHECK-NEXT:	zfhmin  1.0
+// CHECK-NEXT:	zfinx   1.0
+// CHECK-NEXT:	zdinx   1.0
+// CHECK-NEXT:	zca 1.0
+// CHECK-NEXT:	zcb 1.0
+// CHECK-NEXT:	zcd 1.0
+// CHECK-NEXT:	zcf 1.0
+// CHECK-NEXT:	zcmp1.0
+// CHECK-NEXT:	zcmt1.0
+// CHECK-NEXT:	zba 1.0
+// CHECK-NEXT:	zbb

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

2023-07-30 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D156363#4545410 , @phosek wrote:

> We see 14 failing tests on our Linux and macOS builders, and 6 failing tests 
> on Windows builders:
>
> - `Clang :: Driver/csky-toolchain.c`
> - `Clang :: Driver/env.c`
> - `Clang :: Driver/fsanitize.c`
> - `Clang :: Driver/gcc-install-dir.cpp`
> - `Clang :: Driver/gcc-toolchain.cpp`
> - `Clang :: Driver/linux-cross.cpp`
> - `Clang :: Driver/linux-ld.c`
> - `Clang :: Driver/loongarch-toolchain.c`
> - `Clang :: Driver/miamcu-opt.c`
> - `Clang :: Driver/nolibc.c`
> - `Clang :: Driver/pic.c`
> - `Clang :: Driver/riscv32-toolchain.c`
> - `Clang :: Driver/riscv64-toolchain.c`
> - `Clang :: OpenMP/linking.c`
>
> Here is an example of failing builds:
>
> - 
> https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8774104426056321089/overview
> - 
> https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-arm64/b8774104252942379969/overview
> - 
> https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-mac-x64/b8774107666355415825/overview
> - 
> https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8774105312409650497/overview
>
> Can we please revert this change while this is being investigated?

To the best of my knowledge, no official build exhibits these failures.
These are non-official builds with quite complex builds. In the end I'd say I'd 
revert the patch, but my reasoning is different from: the patch broke the 
unofficial bot, so we justify to revert the patch.
It's more on the basis that these tests indicate issues that would likely 
happen on official build bots, if an official build bot had such coverage.

FWIW: I have tried `-DCLANG_DEFAULT_CXX_STDLIB=libc++ -C 
clang/cmake/caches/Fuchsia-stage2.cmake -DLLVM_ENABLE_LTO=off` to be similar to 
your build bots appear to be using (I am unfamiliar with it) but unable to 
reproduce the failures.

I think we need some assistance from Fuchsia to understand why these tests are 
less portable and what adjustment is needed to make them more portable.

---

With that said, `clang/test/Driver/fsanitize.c` is broken on some official 
build bots (e.g. clang-aarch64-sve-vla 
https://lab.llvm.org/buildbot/#/builders/197/builds/8677) and on 
http://45.33.8.238/ (I trust much for representing some configurations that an 
official build bot would cover), so I think a revert is justified, if we don't 
want to unsupport `clang/test/Driver/fsanitize.c`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156363

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

[PATCH] D146054: [RISCV] Add --print-supported-extensions support

2023-07-30 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4340
+}
+
   }

no blank line here



Comment at: clang/test/Driver/print-supported-extensions.c:93
+// CHECK-NEXT: xventanacondops 1.0
+//
+//  CHECK:Experimental extensions

`//CHECK-EMPTY:`



Comment at: clang/test/Driver/print-supported-extensions.c:120
+// CHECK-NEXT: ssaia   1.0
+//
+//  CHECK:Use -march to specify the target's extension.

`//CHECK-EMPTY:`



Comment at: clang/tools/driver/cc1_main.cpp:187
+/// Print supported extensions of the RISCV target.
+static void printSupportedExtensions() {
+  llvm::riscvExtensionsHelp();

The call site should just call riscvExtensionsHelp so that this internal 
linkage function can be avoided.



Comment at: clang/tools/driver/cc1_main.cpp:190
+}
+
+

one blank line



Comment at: clang/tools/driver/cc1_main.cpp:190
+}
+
+

MaskRay wrote:
> one blank line
one blank line



Comment at: llvm/lib/Support/RISCVISAInfo.cpp:217
+  for (const auto &E : SupportedExtensions)
+ExtMap[E.Name] = { E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)

clang-format will remove the space after `{`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

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

[PATCH] D147357: [clang-tidy] Add bugprone-optional-value-conversion check

2023-07-30 Thread Shivam Gupta via Phabricator via cfe-commits

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

LGTM, Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

2023-07-30 Thread Petr Hosek via Phabricator via cfe-commits

phosek added a comment.

We see 14 failing tests on our Linux and macOS builders, and 6 failing tests on 
Windows builders:

- `Clang :: Driver/csky-toolchain.c`
- `Clang :: Driver/env.c`
- `Clang :: Driver/fsanitize.c`
- `Clang :: Driver/gcc-install-dir.cpp`
- `Clang :: Driver/gcc-toolchain.cpp`
- `Clang :: Driver/linux-cross.cpp`
- `Clang :: Driver/linux-ld.c`
- `Clang :: Driver/loongarch-toolchain.c`
- `Clang :: Driver/miamcu-opt.c`
- `Clang :: Driver/nolibc.c`
- `Clang :: Driver/pic.c`
- `Clang :: Driver/riscv32-toolchain.c`
- `Clang :: Driver/riscv64-toolchain.c`
- `Clang :: OpenMP/linking.c`

Here is an example of failing builds:

- 
https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8774104426056321089/overview
- 
https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-arm64/b8774104252942379969/overview
- 
https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-mac-x64/b8774107666355415825/overview
- 
https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8774105312409650497/overview

Can we please revert this change while this is being investigated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156363

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

[PATCH] D156636: [InstCombine] Deduce `align` and `nonnull` return attributes for `llvm.ptrmask`

2023-07-30 Thread Noah Goldstein via Phabricator via cfe-commits

goldstein.w.n created this revision.
goldstein.w.n added reviewers: nikic, RKSimon.
Herald added subscribers: StephenFan, hiraditya.
Herald added a project: All.
goldstein.w.n requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

We can deduce the former based on the mask / incoming pointer
alignment.  We can set the latter based if know the result in non-zero
(this is essentially just caching our analysis result).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156636

Files:
  clang/test/CodeGen/arm64_32-vaarg.c
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/align-addr.ll
  llvm/test/Transforms/InstCombine/consecutive-ptrmask.ll
  llvm/test/Transforms/InstCombine/ptrmask.ll

Index: llvm/test/Transforms/InstCombine/ptrmask.ll
===
--- llvm/test/Transforms/InstCombine/ptrmask.ll
+++ llvm/test/Transforms/InstCombine/ptrmask.ll
@@ -8,7 +8,7 @@
 ; CHECK-LABEL: define ptr @ptrmask_combine_consecutive_preserve_attrs
 ; CHECK-SAME: (ptr [[P0:%.*]], i64 [[M1:%.*]]) {
 ; CHECK-NEXT:[[TMP1:%.*]] = and i64 [[M1]], 224
-; CHECK-NEXT:[[R:%.*]] = call ptr @llvm.ptrmask.p0.i64(ptr [[P0]], i64 [[TMP1]])
+; CHECK-NEXT:[[R:%.*]] = call align 32 ptr @llvm.ptrmask.p0.i64(ptr [[P0]], i64 [[TMP1]])
 ; CHECK-NEXT:ret ptr [[R]]
 ;
   %pm0 = call ptr @llvm.ptrmask.p0.i64(ptr %p0, i64 224)
@@ -31,7 +31,7 @@
 define ptr @ptrmask_combine_consecutive_preserve_attrs_todo0(ptr %p0) {
 ; CHECK-LABEL: define ptr @ptrmask_combine_consecutive_preserve_attrs_todo0
 ; CHECK-SAME: (ptr [[P0:%.*]]) {
-; CHECK-NEXT:[[PM0:%.*]] = call noalias ptr @llvm.ptrmask.p0.i64(ptr [[P0]], i64 224)
+; CHECK-NEXT:[[PM0:%.*]] = call noalias align 32 ptr @llvm.ptrmask.p0.i64(ptr [[P0]], i64 224)
 ; CHECK-NEXT:ret ptr [[PM0]]
 ;
   %pm0 = call noalias ptr @llvm.ptrmask.p0.i64(ptr %p0, i64 224)
@@ -42,7 +42,7 @@
 define ptr @ptrmask_combine_consecutive_preserve_attrs_todo1(ptr %p0) {
 ; CHECK-LABEL: define ptr @ptrmask_combine_consecutive_preserve_attrs_todo1
 ; CHECK-SAME: (ptr [[P0:%.*]]) {
-; CHECK-NEXT:[[PM0:%.*]] = call ptr @llvm.ptrmask.p0.i64(ptr [[P0]], i64 224)
+; CHECK-NEXT:[[PM0:%.*]] = call align 32 ptr @llvm.ptrmask.p0.i64(ptr [[P0]], i64 224)
 ; CHECK-NEXT:ret ptr [[PM0]]
 ;
   %pm0 = call ptr @llvm.ptrmask.p0.i64(ptr %p0, i64 224)
@@ -53,7 +53,7 @@
 define ptr @ptrmask_combine_consecutive_preserve_attrs_todo2(ptr %p0) {
 ; CHECK-LABEL: define ptr @ptrmask_combine_consecutive_preserve_attrs_todo2
 ; CHECK-SAME: (ptr [[P0:%.*]]) {
-; CHECK-NEXT:[[PM0:%.*]] = call noalias ptr @llvm.ptrmask.p0.i32(ptr [[P0]], i32 224)
+; CHECK-NEXT:[[PM0:%.*]] = call noalias align 32 ptr @llvm.ptrmask.p0.i32(ptr [[P0]], i32 224)
 ; CHECK-NEXT:ret ptr [[PM0]]
 ;
   %pm0 = call noalias ptr @llvm.ptrmask.p0.i32(ptr %p0, i32 224)
@@ -64,9 +64,9 @@
 define ptr @ptrmask_combine_add_nonnull(ptr %p) {
 ; CHECK-LABEL: define ptr @ptrmask_combine_add_nonnull
 ; CHECK-SAME: (ptr [[P:%.*]]) {
-; CHECK-NEXT:[[PM0:%.*]] = call ptr @llvm.ptrmask.p0.i64(ptr [[P]], i64 -64)
+; CHECK-NEXT:[[PM0:%.*]] = call align 64 ptr @llvm.ptrmask.p0.i64(ptr [[P]], i64 -64)
 ; CHECK-NEXT:[[PGEP:%.*]] = getelementptr i8, ptr [[PM0]], i64 33
-; CHECK-NEXT:[[R:%.*]] = call ptr @llvm.ptrmask.p0.i64(ptr [[PGEP]], i64 -16)
+; CHECK-NEXT:[[R:%.*]] = call nonnull align 32 ptr @llvm.ptrmask.p0.i64(ptr [[PGEP]], i64 -16)
 ; CHECK-NEXT:ret ptr [[R]]
 ;
   %pm0 = call ptr @llvm.ptrmask.p0.i64(ptr %p, i64 -64)
@@ -78,7 +78,7 @@
 define ptr @ptrmask_combine_add_alignment(ptr %p) {
 ; CHECK-LABEL: define ptr @ptrmask_combine_add_alignment
 ; CHECK-SAME: (ptr [[P:%.*]]) {
-; CHECK-NEXT:[[R:%.*]] = call ptr @llvm.ptrmask.p0.i64(ptr [[P]], i64 -64)
+; CHECK-NEXT:[[R:%.*]] = call align 64 ptr @llvm.ptrmask.p0.i64(ptr [[P]], i64 -64)
 ; CHECK-NEXT:ret ptr [[R]]
 ;
   %r = call ptr @llvm.ptrmask.p0.i64(ptr %p, i64 -64)
@@ -88,7 +88,7 @@
 define ptr @ptrmask_combine_add_alignment2(ptr align 32 %p) {
 ; CHECK-LABEL: define ptr @ptrmask_combine_add_alignment2
 ; CHECK-SAME: (ptr align 32 [[P:%.*]]) {
-; CHECK-NEXT:[[R:%.*]] = call ptr @llvm.ptrmask.p0.i32(ptr [[P]], i32 -64)
+; CHECK-NEXT:[[R:%.*]] = call align 64 ptr @llvm.ptrmask.p0.i32(ptr [[P]], i32 -64)
 ; CHECK-NEXT:ret ptr [[R]]
 ;
   %r = call ptr @llvm.ptrmask.p0.i32(ptr %p, i32 -64)
@@ -98,7 +98,7 @@
 define ptr @ptrmask_combine_improve_alignment(ptr %p) {
 ; CHECK-LABEL: define ptr @ptrmask_combine_improve_alignment
 ; CHECK-SAME: (ptr [[P:%.*]]) {
-; CHECK-NEXT:[[R:%.*]] = call align 32 ptr @llvm.ptrmask.p0.i32(ptr [[P]], i32 -64)
+; CHECK-NEXT:[[R:%.*]] = call align 64 ptr @llvm.ptrmask.p0.i32(ptr [[P]], i32 -64)
 ; CHECK-NEXT:ret ptr [[R]]
 ;
   %r = call align 32 ptr @llvm.ptrmask.p0.i32(ptr %p, i32 -64)
Index: llvm/test/Transforms/InstCombine/consecutive

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

2023-07-30 Thread Douglas Yung via Phabricator via cfe-commits

dyung added a comment.

If you cannot reproduce this issue, I can take my buildbot offline and try to 
reproduce it to pinpoint which command is causing the problem, but I won’t be 
able to do so until Monday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156363

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

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

2023-07-30 Thread Douglas Yung via Phabricator via cfe-commits

dyung added a comment.

In D156363#4544803 , @aaron.ballman 
wrote:

> In D156363#4544782 , @thakis wrote:
>
>> http://45.33.8.238/linux/113921/step_7.txt is failing due to this. Is this 
>> something on my end?
>
> Nope, this same failure is happening in our build farm:
> https://lab.llvm.org/buildbot/#/builders/197/builds/8677
> https://lab.llvm.org/buildbot/#/builders/179/builds/6872
> https://lab.llvm.org/buildbot/#/builders/198/builds/4171
>
> (and others)

We are also seeing this on our build bot: 
https://lab.llvm.org/buildbot/#/builders/247/builds/7117


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156363

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

[PATCH] D141892: [clang-tidy] Implement modernize-use-constraints

2023-07-30 Thread Piotr Zegar via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG893d53d11c01: [clang-tidy] Implement 
modernize-use-constraints (authored by ccotter, committed by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141892

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.h
  clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
  clang-tools-extra/clang-tidy/utils/LexerUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/use-constraints.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints-first-greatergreater.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp
@@ -0,0 +1,726 @@
+// RUN: %check_clang_tidy -std=c++20 %s modernize-use-constraints %t -- -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+template  struct enable_if { };
+
+template  struct enable_if { typedef T type; };
+
+template 
+using enable_if_t = typename enable_if::type;
+
+} // namespace std
+// NOLINTEND
+
+template 
+struct ConsumeVariadic;
+
+struct Obj {
+};
+
+namespace enable_if_in_return_type {
+
+
+// Section 1: enable_if in return type of function
+
+
+
+// General tests
+
+
+template 
+typename std::enable_if::type basic() {
+  return Obj{};
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}Obj basic() requires T::some_value {{{$}}
+
+template 
+std::enable_if_t basic_t() {
+  return Obj{};
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}Obj basic_t() requires T::some_value {{{$}}
+
+template 
+auto basic_trailing() -> typename std::enable_if::type {
+  return Obj{};
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:26: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}auto basic_trailing() -> Obj requires T::some_value {{{$}}
+
+template 
+typename std::enable_if::type existing_constraint() requires (T::another_value) {
+  return Obj{};
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}typename std::enable_if::type existing_constraint() requires (T::another_value) {{{$}}
+
+template 
+typename std::enable_if::type decl_without_def();
+
+template 
+typename std::enable_if::type decl_with_separate_def();
+
+template 
+typename std::enable_if::type decl_with_separate_def() {
+  return Obj{};
+}
+// FIXME - Support definitions with separate decls
+
+template 
+std::enable_if_t no_dependent_type(U) {
+  return Obj{};
+}
+// FIXME - Support non-dependent enable_ifs. Low priority though...
+
+template 
+typename std::enable_if::type* pointer_of_enable_if() {
+  return nullptr;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}int* pointer_of_enable_if() requires T::some_value {{{$}}
+
+template 
+std::enable_if_t* pointer_of_enable_if_t() {
+  return nullptr;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}int* pointer_of_enable_if_t() requires T::some_value {{{$}}
+
+template 
+const std::enable_if_t* const_pointer_of_enable_if_t() {
+  return nullptr;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:7: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}const int* const_pointer_of_enable_if_t() requires T::some_value {{{$}}
+
+template 
+std::enable_if_t const * const_pointer_of_enable_if_t2() {
+  return nullptr;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}int const * const_pointer_of_enable_if_t2() requires T::some_value {{{$}}
+
+
+template 
+std::enable_if_t& reference_of_enable_if_t() {
+  static int x; return x;
+}
+// CHECK-ME

[clang-tools-extra] 893d53d - [clang-tidy] Implement modernize-use-constraints

2023-07-30 Thread Piotr Zegar via cfe-commits


Author: Chris Cotter
Date: 2023-07-30T20:43:18Z
New Revision: 893d53d11c0116718ecc816263f12d51c4612e1d

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

LOG: [clang-tidy] Implement modernize-use-constraints

Add new check to replace enable_if with C++20 constraints

Reviewed By: PiotrZSL

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

Added: 
clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.h
clang-tools-extra/docs/clang-tidy/checks/modernize/use-constraints.rst

clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints-first-greatergreater.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp

Modified: 
clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
clang-tools-extra/clang-tidy/utils/LexerUtils.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index ef10cb0e388264..a14501d13930e0 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -29,6 +29,7 @@ add_clang_library(clangTidyModernizeModule
   UnaryStaticAssertCheck.cpp
   UseAutoCheck.cpp
   UseBoolLiteralsCheck.cpp
+  UseConstraintsCheck.cpp
   UseDefaultMemberInitCheck.cpp
   UseEmplaceCheck.cpp
   UseEqualsDefaultCheck.cpp

diff  --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp 
b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index 9bbc4dc50ec0e4..73751cf2705068 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -30,6 +30,7 @@
 #include "UnaryStaticAssertCheck.h"
 #include "UseAutoCheck.h"
 #include "UseBoolLiteralsCheck.h"
+#include "UseConstraintsCheck.h"
 #include "UseDefaultMemberInitCheck.h"
 #include "UseEmplaceCheck.h"
 #include "UseEqualsDefaultCheck.h"
@@ -85,6 +86,8 @@ class ModernizeModule : public ClangTidyModule {
 CheckFactories.registerCheck("modernize-use-auto");
 CheckFactories.registerCheck(
 "modernize-use-bool-literals");
+CheckFactories.registerCheck(
+"modernize-use-constraints");
 CheckFactories.registerCheck(
 "modernize-use-default-member-init");
 CheckFactories.registerCheck("modernize-use-emplace");

diff  --git a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
new file mode 100644
index 00..951cc2f9903ba3
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
@@ -0,0 +1,488 @@
+//===--- UseConstraintsCheck.cpp - clang-tidy 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "UseConstraintsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include "../utils/LexerUtils.h"
+
+#include 
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+struct EnableIfData {
+  TemplateSpecializationTypeLoc Loc;
+  TypeLoc Outer;
+};
+
+namespace {
+AST_MATCHER(FunctionDecl, hasOtherDeclarations) {
+  auto It = Node.redecls_begin();
+  auto EndIt = Node.redecls_end();
+
+  if (It == EndIt)
+return false;
+
+  ++It;
+  return It != EndIt;
+}
+} // namespace
+
+void UseConstraintsCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  functionTemplateDecl(
+  has(functionDecl(unless(hasOtherDeclarations()), isDefinition(),
+   hasReturnTypeLoc(typeLoc().bind("return")))
+  .bind("function")))
+  .bind("functionTemplate"),
+  this);
+}
+
+static std::optional
+matchEnableIfSpecializationImplTypename(TypeLoc TheType) {
+  if (const auto Dep = TheType.getAs()) {
+const IdentifierInfo *Identifier = Dep.getTypePtr()->getIdentifier();
+if (!Identifier || Identifier->getName() != "type" ||
+Dep.getTypePtr()->getKeyword() != ETK_Typename) {
+  return std::nullopt;
+}
+TheType = Dep.getQualifierLoc().getTypeLoc();
+  }
+
+  if (const auto SpecializationLoc =
+  TheType.getAs()) {
+
+const a

[PATCH] D141892: [clang-tidy] Implement modernize-use-constraints

2023-07-30 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL accepted this revision.
PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp:42
+void UseConstraintsCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  functionTemplateDecl(

Would be good to exclude here implicit code. For example with 
IgnoreUnlessSpelledInSource, maybe in next patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141892

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

[PATCH] D156624: [clang-tidy] Access checks not done classes derived of std::array

2023-07-30 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added a comment.

In D156624#4545170 , @sousajo wrote:

> Good point, I cannot think of an _easy_ way to check it.

I think you can do something like this:

  Finder->addMatcher(
  cxxOperatorCallExpr(
  hasOverloadedOperatorName("[]"),
  
calle(cxxMethodDecl(ofClass(cxxRecordDecl(hasName("::std::array")).bind("type"
  hasArgument(1, expr().bind("index")))
  .bind("expr"),
  this);

In such case instead of checking argument passed to call, we check what 
operator we used and who provided it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156624

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

[PATCH] D156624: [clang-tidy] Access checks not done classes derived of std::array

2023-07-30 Thread Jorge Pinto Sousa via Phabricator via cfe-commits

sousajo added a comment.

In D156624#4545107 , @PiotrZSL wrote:

> Good, just verify that documentation for this check is still proper, and add 
> entry in release notes about this change.

I missed those, I will sure ^^

In D156624#4545117 , @carlosgalvezp 
wrote:

> What about the use case of privately inheriting `std::array`, and overriding 
> the `operator[]` there with bounds check? I believe there shouldn't be 
> warnings there.
>
> Would it be possible to check only _public_ inheritance?

Good point, I cannot think of an _easy_ way to check it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156624

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

[PATCH] D147357: [clang-tidy] Add bugprone-optional-value-conversion check

2023-07-30 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL updated this revision to Diff 545464.
PiotrZSL added a comment.

Review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
@@ -0,0 +1,213 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-optional-value-conversion %t -- --fix-notes
+// RUN: %check_clang_tidy -check-suffix=CUSTOM -std=c++17-or-later %s bugprone-optional-value-conversion %t -- \
+// RUN: -config="{CheckOptions: [{key: 'bugprone-optional-value-conversion.OptionalTypes', value: 'CustomOptional'}, \
+// RUN:  {key: 'bugprone-optional-value-conversion.ValueMethods', value: '::Read$;::Ooo$'}]}" --fix-notes
+
+namespace std {
+  template
+  struct optional
+  {
+constexpr optional() noexcept;
+constexpr optional(T&&) noexcept;
+constexpr optional(const T&) noexcept;
+template
+constexpr optional(U&&) noexcept;
+const T& operator*() const;
+T* operator->();
+const T* operator->() const;
+T& operator*();
+const T& value() const;
+T& value();
+const T& get() const;
+T& get();
+T value_or(T) const;
+  };
+
+  template 
+  T&& move(T &x) {
+return static_cast(x);
+  }
+}
+
+namespace boost {
+  template
+  struct optional {
+constexpr optional() noexcept;
+constexpr optional(const T&) noexcept;
+const T& operator*() const;
+const T& get() const;
+  };
+}
+
+namespace absl {
+  template
+  struct optional {
+constexpr optional() noexcept;
+constexpr optional(const T&) noexcept;
+const T& operator*() const;
+const T& value() const;
+  };
+}
+
+template
+struct CustomOptional {
+  CustomOptional();
+  CustomOptional(const T&);
+  const T& Read() const;
+  T& operator*();
+  T& Ooo();
+};
+
+void takeOptionalValue(std::optional);
+void takeOptionalRef(const std::optional&);
+void takeOptionalRRef(std::optional&&);
+void takeOtherOptional(std::optional);
+void takeBOptionalValue(boost::optional);
+void takeAOptionalValue(absl::optional);
+
+void incorrect(std::optional param)
+{
+  std::optional* ptr = ¶m;
+  takeOptionalValue(**ptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalValue(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(param.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(ptr->value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalValue(param.operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(ptr->operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalRef(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(param);
+  takeOptionalRef(param.value());
+  // CHECK-MESSAGES:

[PATCH] D156624: [clang-tidy] Access checks not done classes derived of std::array

2023-07-30 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp added a comment.

What about the use case of privately inheriting `std::array`, and overriding 
the `operator[]` there with bounds check? Would it be possible to check only 
_public_ inheritance?


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

https://reviews.llvm.org/D156624

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

[PATCH] D156624: [clang-tidy] Access checks not done classes derived of std::array

2023-07-30 Thread Jorge Pinto Sousa via Phabricator via cfe-commits

sousajo added a comment.

hi @PiotrZSL, thanks for the review :) can you please land it for me?

Jorge Pinto Sousa 


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

https://reviews.llvm.org/D156624

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

[PATCH] D156624: [clang-tidy] Access checks not done classes derived of std::array

2023-07-30 Thread Piotr Zegar via Phabricator via cfe-commits

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

LGTM


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

https://reviews.llvm.org/D156624

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

[PATCH] D156624: [clang-tidy] Access checks not done classes derived of std::array

2023-07-30 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added a comment.

Good, just verify that documentation for this check is still proper, and add 
entry in release notes about this change.


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

https://reviews.llvm.org/D156624

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

[PATCH] D147357: [clang-tidy] Add bugprone-optional-value-conversion check

2023-07-30 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp:61
+
matchers::matchesAnyListedName(ValueMethods)
+  .bind("fun")),
+  hasType(qualType().bind("value-type")));

xgupta wrote:
> Any good reason to choose "fun" as the matcher name?
shortcut from function, as its matcher for a cxxMemberCallExpr i will change it 
to for example "call"



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst:32
+}
+
+Value extraction using ``operator *`` is matched by default.

xgupta wrote:
> Can we also write - A better approach would be to directly pass opt to the 
> print function without extracting its value: 
> `print(opt)`
Yes I will add such thing...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

[PATCH] D156624: [clang-tidy] Access checks not done classes derived of std::array

2023-07-30 Thread Jorge Pinto Sousa via Phabricator via cfe-commits

sousajo created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, arphaman, kbarton, 
xazax.hun, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
sousajo requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Index accessing checks are not performed for derived classes of
of `std::array`, as only `std::array` itself and its aliases
seems to be checked.

  

This patch aims to extend it for derived classes such as:

  template
  class DerivedArray : public std::array {};


https://reviews.llvm.org/D156624

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
@@ -23,6 +23,9 @@
   return base + 3;
 }
 
+template
+class DerivedArray : public std::array {};
+
 void f(std::array a, int pos) {
   a [ pos / 2 /*comment*/] = 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when 
the index is not an integer constant expression 
[cppcoreguidelines-pro-bounds-constant-array-index]
@@ -68,6 +71,51 @@
   m[const_index(6)] = 3; // OK, constant index and inside bounds
 }
 
+void f_derived(DerivedArray a, int pos) {
+  a [ pos / 2 /*comment*/] = 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when 
the index is not an integer constant expression 
[cppcoreguidelines-pro-bounds-constant-array-index]
+  int j = a[pos - 1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when 
the index is not an integer constant expression
+
+  a.at(pos-1) = 2; // OK, at() instead of []
+  gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of []
+
+  a[-1] = 3;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index -1 is 
negative [cppcoreguidelines-pro-bounds-constant-array-index]
+  a[10] = 4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past 
the end of the array (which contains 10 elements) 
[cppcoreguidelines-pro-bounds-constant-array-index]
+
+  a[const_index(7)] = 3;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past 
the end of the array (which contains 10 elements)
+
+  a[0] = 3; // OK, constant index and inside bounds
+  a[1] = 3; // OK, constant index and inside bounds
+  a[9] = 3; // OK, constant index and inside bounds
+  a[const_index(6)] = 3; // OK, constant index and inside bounds
+
+  using MyArray = DerivedArray;
+  MyArray m{};
+  m [ pos / 2 /*comment*/] = 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when 
the index is not an integer constant expression 
[cppcoreguidelines-pro-bounds-constant-array-index]
+  int jj = m[pos - 1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use array subscript when 
the index is not an integer constant expression
+
+  m.at(pos-1) = 2; // OK, at() instead of []
+  gsl::at(m, pos-1) = 2; // OK, gsl::at() instead of []
+  m[-1] = 3;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index -1 is 
negative [cppcoreguidelines-pro-bounds-constant-array-index]
+  m[10] = 4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past 
the end of the array (which contains 10 elements) 
[cppcoreguidelines-pro-bounds-constant-array-index]
+
+  m[const_index(7)] = 3;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past 
the end of the array (which contains 10 elements)
+
+  m[0] = 3; // OK, constant index and inside bounds
+  m[1] = 3; // OK, constant index and inside bounds
+  m[9] = 3; // OK, constant index and inside bounds
+  m[const_index(6)] = 3; // OK, constant index and inside bounds
+}
+
 void g() {
   int a[10];
   for (int i = 0; i < 10; ++i) {
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -50,7 +50,8 @@
   hasOverloadedOperatorName("[]"),
   hasArgument(
   0, hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration(
- cxxRecordDecl(hasName("::std::array")).bind("type")),
+ 
cxxRecordDecl(isSameOrDerivedFrom(hasName("::std::array")))
+ .bind("type")),
   hasArgument(1, expr().bind("index")))
   .bind("expr"),
   this);


Index: cl

[PATCH] D147357: [clang-tidy] Add bugprone-optional-value-conversion check

2023-07-30 Thread Shivam Gupta via Phabricator via cfe-commits

xgupta added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp:61
+
matchers::matchesAnyListedName(ValueMethods)
+  .bind("fun")),
+  hasType(qualType().bind("value-type")));

Any good reason to choose "fun" as the matcher name?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst:32
+}
+
+Value extraction using ``operator *`` is matched by default.

Can we also write - A better approach would be to directly pass opt to the 
print function without extracting its value: 
`print(opt)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

[PATCH] D141892: [clang-tidy] Implement modernize-use-constraints

2023-07-30 Thread Chris Cotter via Phabricator via cfe-commits

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

> 3. What about boost enable_if ? Support it, or restrict check to std only.
> 4. What about enable_if used as an function argument ? Support it, or add 
> some info to documentation that such construction is not supported.

I added some notes to the documentation on supported constructions and 
unsupported ones. The unsupported ones can be added; I intentionally did not 
support them in this initial version of the tool to keep the review simpler, 
although I'd like to add them soon after it lands.

If there are no other comments, @PiotrZSL would you mind landing this for me? 
Thanks!

Comment at: clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp:352
+  FixIts.push_back(FixItHint::CreateInsertion(
+  *ConstraintInsertionLoc, "requires " + *ConditionText + " "));
+  return FixIts;

PiotrZSL wrote:
> You not checking anywhere if this optional is initialized.
Indeed - `bugprone-unchecked-optional-access` finds this!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141892

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

[PATCH] D141892: [clang-tidy] Implement modernize-use-constraints

2023-07-30 Thread Chris Cotter via Phabricator via cfe-commits

ccotter updated this revision to Diff 545450.
ccotter marked 5 inline comments as done.
ccotter added a comment.

- Improve docs, check for null/empty


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141892

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.h
  clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
  clang-tools-extra/clang-tidy/utils/LexerUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/use-constraints.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints-first-greatergreater.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp
@@ -0,0 +1,726 @@
+// RUN: %check_clang_tidy -std=c++20 %s modernize-use-constraints %t -- -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+template  struct enable_if { };
+
+template  struct enable_if { typedef T type; };
+
+template 
+using enable_if_t = typename enable_if::type;
+
+} // namespace std
+// NOLINTEND
+
+template 
+struct ConsumeVariadic;
+
+struct Obj {
+};
+
+namespace enable_if_in_return_type {
+
+
+// Section 1: enable_if in return type of function
+
+
+
+// General tests
+
+
+template 
+typename std::enable_if::type basic() {
+  return Obj{};
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}Obj basic() requires T::some_value {{{$}}
+
+template 
+std::enable_if_t basic_t() {
+  return Obj{};
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}Obj basic_t() requires T::some_value {{{$}}
+
+template 
+auto basic_trailing() -> typename std::enable_if::type {
+  return Obj{};
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:26: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}auto basic_trailing() -> Obj requires T::some_value {{{$}}
+
+template 
+typename std::enable_if::type existing_constraint() requires (T::another_value) {
+  return Obj{};
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}typename std::enable_if::type existing_constraint() requires (T::another_value) {{{$}}
+
+template 
+typename std::enable_if::type decl_without_def();
+
+template 
+typename std::enable_if::type decl_with_separate_def();
+
+template 
+typename std::enable_if::type decl_with_separate_def() {
+  return Obj{};
+}
+// FIXME - Support definitions with separate decls
+
+template 
+std::enable_if_t no_dependent_type(U) {
+  return Obj{};
+}
+// FIXME - Support non-dependent enable_ifs. Low priority though...
+
+template 
+typename std::enable_if::type* pointer_of_enable_if() {
+  return nullptr;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}int* pointer_of_enable_if() requires T::some_value {{{$}}
+
+template 
+std::enable_if_t* pointer_of_enable_if_t() {
+  return nullptr;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}int* pointer_of_enable_if_t() requires T::some_value {{{$}}
+
+template 
+const std::enable_if_t* const_pointer_of_enable_if_t() {
+  return nullptr;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:7: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}const int* const_pointer_of_enable_if_t() requires T::some_value {{{$}}
+
+template 
+std::enable_if_t const * const_pointer_of_enable_if_t2() {
+  return nullptr;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}int const * const_pointer_of_enable_if_t2() requires T::some_value {{{$}}
+
+
+template 
+std::enable_if_t& reference_of_enable_if_t() {
+  static int x; return x;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

2023-07-30 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D156363#4544803 , @aaron.ballman 
wrote:

> In D156363#4544782 , @thakis wrote:
>
>> http://45.33.8.238/linux/113921/step_7.txt is failing due to this. Is this 
>> something on my end?
>
> Nope, this same failure is happening in our build farm:
> https://lab.llvm.org/buildbot/#/builders/197/builds/8677
> https://lab.llvm.org/buildbot/#/builders/179/builds/6872
> https://lab.llvm.org/buildbot/#/builders/198/builds/4171
>
> (and others)

`test/Driver/fsanitize.c` contains many `%clang` RUN lines. Presumably one 
somehow exits with code 1 , but without using `llvm-lit -vv` we don't know 
which command fails, making this almost impossible to investigate...

I guess adding `--dump-input=fail` to every FileCheck command will be useful as 
well but it would cluster up the history of `fsanitize.c`...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156363

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

[PATCH] D156616: [clang-tidy] Fix c_str() removal and cast addition when re-ordering arguments

2023-07-30 Thread Mike Crowe via Phabricator via cfe-commits

mikecrowe updated this revision to Diff 545446.
mikecrowe added a comment.

Rebase on top of D156616  and add more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156616

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h
  clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
  clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
  clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp
@@ -0,0 +1,97 @@
+// RUN: %check_clang_tidy \
+// RUN:   -std=c++20 %s modernize-use-std-format %t -- \
+// RUN:   -config="{CheckOptions: [{key: StrictMode, value: true}]}" \
+// RUN:   -- -isystem %clang_tidy_headers
+// RUN: %check_clang_tidy \
+// RUN:   -std=c++20 %s modernize-use-std-format %t -- \
+// RUN:   -config="{CheckOptions: [{key: StrictMode, value: false}]}" \
+// RUN:   -- -isystem %clang_tidy_headers
+#include 
+// CHECK-FIXES: #include 
+
+namespace absl
+{
+// Use const char * for the format since the real type is hard to mock up.
+template 
+std::string StrFormat(const char *format, const Args&... args);
+} // namespace absl
+
+template 
+struct iterator {
+  T *operator->();
+  T &operator*();
+};
+
+std::string StrFormat_simple() {
+  return absl::StrFormat("Hello");
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format]
+  // CHECK-FIXES: return std::format("Hello");
+}
+
+std::string StrFormat_complex(const char *name, double value) {
+  return absl::StrFormat("'%s'='%f'", name, value);
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format]
+  // CHECK-FIXES: return std::format("'{}'='{:f}'", name, value);
+}
+
+std::string StrFormat_integer_conversions() {
+  return absl::StrFormat("int:%d int:%d char:%c char:%c", 65, 'A', 66, 'B');
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format]
+  // CHECK-FIXES: return std::format("int:{} int:{:d} char:{:c} char:{}", 65, 'A', 66, 'B');
+}
+
+// FormatConverter is capable of removing newlines from the end of the format
+// string. Ensure that isn't incorrectly happening for std::format.
+std::string StrFormat_no_newline_removal() {
+  return absl::StrFormat("a line\n");
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format]
+  // CHECK-FIXES: return std::format("a line\n");
+}
+
+// FormatConverter is capable of removing newlines from the end of the format
+// string. Ensure that isn't incorrectly happening for std::format.
+std::string StrFormat_cstr_removal(const std::string &s1, const std::string *s2) {
+  return absl::StrFormat("%s %s %s %s", s1.c_str(), s1.data(), s2->c_str(), s2->data());
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format]
+  // CHECK-FIXES: return std::format("{} {} {} {}", s1, s1, *s2, *s2);
+}
+
+std::string StrFormat_strict_conversion() {
+  const unsigned char uc = 'A';
+  return absl::StrFormat("Integer %hhd from unsigned char\n", uc);
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format]
+  // CHECK-FIXES: return std::format("Integer {} from unsigned char\n", uc);
+}
+
+std::string StrFormat_field_width_and_precision() {
+  auto s1 = absl::StrFormat("width only:%*d width and precision:%*.*f precision only:%.*f", 3, 42, 4, 2, 3.14159265358979323846, 5, 2.718);
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format]
+  // CHECK-FIXES: std::format("width only:{:{}} width and precision:{:{}.{}f} precision only:{:.{}f}", 42, 3, 3.14159265358979323846, 4, 2, 2.718, 5);
+
+  auto s2 = absl::StrFormat("width and precision positional:%1$*2$.*3$f after", 3.14159265358979323846, 4, 2);
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format]
+  // CHECK-FIXES: std::format("width and precision positional

[PATCH] D156616: [clang-tidy] Fix c_str() removal and cast addition when re-ordering arguments

2023-07-30 Thread Mike Crowe via Phabricator via cfe-commits

mikecrowe added a comment.

Fixing this was rather messier than I expected, but this solution does seem to 
work for the test cases. Here's my original commit message before I trimmed it, 
in case it provides any more insight.

  [clang-tidy] Fix c_str() removal and cast addition when re-ordering arguments
  
  The modernize-use-std-print check would get confused if it had to
  re-order field-width and precision arguments at the same time as adding
  casts or removing calls to c_str(). For example applying the check with
  StrictMode enabled to:
  
   printf("%*s=%*d\n", 4, s.c_str(), 3, ui);
  
  yields:
  
   std::println("{:>{}}={:{}}", s.c_str(), s4, ui, static_cast(3));
  
  rather than the expected:
  
   std::println("{:>{}}={:{}}", s, 4, static_cast(ui), 3);
  
  Fix this by:
  
  - storing the ArgIndex rather than the Expr pointer for any arguments
that need casts to be added in ArgFixes so that the index can be
modified when the arguments are re-ordered. Use a struct rather than a
tuple for clarity.
  
  - Making applyFixes do argument re-ordering first, but also taking care
of c_str() removal at the same time if necessary. Update the argument
index of any ArgFixes too.
  
  - Apply the ArgFixes afterwards.
  
  - Finally apply and c_str() removals that remain.
  
  - Add lit check test cases for the combinations of argument reordering,
casts and c_str() removal. This required moving the definition of
struct iterator earlier in use-std-print.cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156616

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

[PATCH] D156616: [clang-tidy] Fix c_str() removal and cast addition when re-ordering arguments

2023-07-30 Thread Mike Crowe via Phabricator via cfe-commits

mikecrowe created this revision.
mikecrowe added a reviewer: PiotrZSL.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
mikecrowe requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The modernize-use-std-print check would get confused if it had to
re-order field-width and precision arguments at the same time as adding
casts or removing calls to c_str().

Fix this by tracking the argument indices and combining c_str() removal
with argument re-ordering. Add missing test cases to lit check.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156616

Files:
  clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
  clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
@@ -14,6 +14,12 @@
 #include 
 #include 
 
+template 
+struct iterator {
+  T *operator->();
+  T &operator*();
+};
+
 void printf_simple() {
   printf("Hello");
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
@@ -1121,11 +1127,32 @@
   // CHECK-FIXES: std::println("Hello {:.5}", 'G');
 }
 
-void printf_field_width_and_precision() {
+void printf_field_width_and_precision(const std::string &s1, const std::string &s2, const std::string &s3)
+{
   printf("width only:%*d width and precision:%*.*f precision only:%.*f\n", 3, 42, 4, 2, 3.14159265358979323846, 5, 2.718);
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
   // CHECK-FIXES: std::println("width only:{:{}} width and precision:{:{}.{}f} precision only:{:.{}f}", 42, 3, 3.14159265358979323846, 4, 2, 2.718, 5);
 
+  const unsigned int ui1 = 42, ui2 = 43, ui3 = 44;
+  printf("casts width only:%*d width and precision:%*.*d precision only:%.*d\n", 3, ui1, 4, 2, ui2, 5, ui3);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES-NOTSTRICT: std::println("casts width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", ui1, 3, ui2, 4, 2, ui3, 5);
+  // CHECK-FIXES-STRICT: std::println("casts width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", static_cast(ui1), 3, static_cast(ui2), 4, 2, static_cast(ui3), 5);
+
+  printf("c_str removal width only:%*s width and precision:%*.*s precision only:%.*s\n", 3, s1.c_str(), 4, 2, s2.c_str(), 5, s3.c_str());
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println("c_str removal width only:{:>{}} width and precision:{:>{}.{}} precision only:{:.{}}", s1, 3, s2, 4, 2, s3, 5);
+
+  const std::string *ps1 = &s1, *ps2 = &s2, *ps3 = &s3;
+  printf("c_str() removal pointer width only:%-*s width and precision:%-*.*s precision only:%-.*s\n", 3, ps1->c_str(), 4, 2, ps2->c_str(), 5, ps3->c_str());
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println("c_str() removal pointer width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", *ps1, 3, *ps2, 4, 2, *ps3, 5);
+
+  iterator is1, is2, is3;
+  printf("c_str() removal iterator width only:%-*s width and precision:%-*.*s precision only:%-.*s\n", 3, is1->c_str(), 4, 2, is2->c_str(), 5, is3->c_str());
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println("c_str() removal iterator width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", *is1, 3, *is2, 4, 2, *is3, 5);
+
   printf("width and precision positional:%1$*2$.*3$f after\n", 3.14159265358979323846, 4, 2);
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
   // CHECK-FIXES: std::println("width and precision positional:{0:{1}.{2}f} after", 3.14159265358979323846, 4, 2);
@@ -1134,9 +1161,13 @@
   printf("width only:%3$*1$d width and precision:%4$*1$.*2$f precision only:%5$.*2$f\n", width, precision, 42, 3.1415926, 2.718);
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
   // CHECK-FIXES: std::println("width only:{2:{0}} width and precision:{3:{0}.{1}f} precision only:{4:.{1}f}", width, precision, 42, 3.1415926, 2.718);
+
+  printf("c_str removal width only:%3$*1$s width and precision:%4$*1$.*2$s precision only:%5$.*2$s\n", width, precision, s1.c_str(), s2.c_str(), s3.c_str(

[PATCH] D149015: [clang-tidy] Added bugprone-inc-dec-in-conditions check

2023-07-30 Thread Piotr Zegar via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf27f22b34516: [clang-tidy] Added 
bugprone-inc-dec-in-conditions check (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149015

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/IncDecInConditionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/IncDecInConditionsCheck.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/Matchers.cpp
  clang-tools-extra/clang-tidy/utils/Matchers.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/inc-dec-in-conditions.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/inc-dec-in-conditions.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/inc-dec-in-conditions.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/inc-dec-in-conditions.cpp
@@ -0,0 +1,70 @@
+// RUN: %check_clang_tidy %s bugprone-inc-dec-in-conditions %t
+
+template
+struct Iterator {
+  Iterator operator++(int);
+  Iterator operator--(int);
+  Iterator& operator++();
+  Iterator& operator--();
+  T operator*();
+  bool operator==(Iterator) const;
+  bool operator!=(Iterator) const;
+};
+
+template
+struct Container {
+  Iterator begin();
+  Iterator end();
+};
+
+bool f(int x) {
+  return (++x != 5 or x == 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: incrementing and referencing a variable in a complex condition can cause unintended side-effects due to C++'s order of evaluation, consider moving the modification outside of the condition to avoid misunderstandings [bugprone-inc-dec-in-conditions]
+}
+
+bool f2(int x) {
+  return (x++ != 5 or x == 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: incrementing and referencing a variable in a complex condition can cause unintended side-effects due to C++'s order of evaluation, consider moving the modification outside of the condition to avoid misunderstandings [bugprone-inc-dec-in-conditions]
+}
+
+bool c(Container x) {
+  auto it = x.begin();
+  return (it++ != x.end() and *it);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: incrementing and referencing a variable in a complex condition can cause unintended side-effects due to C++'s order of evaluation, consider moving the modification outside of the condition to avoid misunderstandings [bugprone-inc-dec-in-conditions]
+}
+
+bool c2(Container x) {
+  auto it = x.begin();
+  return (++it != x.end() and *it);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: incrementing and referencing a variable in a complex condition can cause unintended side-effects due to C++'s order of evaluation, consider moving the modification outside of the condition to avoid misunderstandings [bugprone-inc-dec-in-conditions]
+}
+
+bool d(int x) {
+  return (--x != 5 or x == 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: decrementing and referencing a variable in a complex condition can cause unintended side-effects due to C++'s order of evaluation, consider moving the modification outside of the condition to avoid misunderstandings [bugprone-inc-dec-in-conditions]
+}
+
+bool d2(int x) {
+  return (x-- != 5 or x == 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: decrementing and referencing a variable in a complex condition can cause unintended side-effects due to C++'s order of evaluation, consider moving the modification outside of the condition to avoid misunderstandings [bugprone-inc-dec-in-conditions]
+}
+
+bool g(Container x) {
+  auto it = x.begin();
+  return (it-- != x.end() and *it);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: decrementing and referencing a variable in a complex condition can cause unintended side-effects due to C++'s order of evaluation, consider moving the modification outside of the condition to avoid misunderstandings [bugprone-inc-dec-in-conditions]
+}
+
+bool g2(Container x) {
+  auto it = x.begin();
+  return (--it != x.end() and *it);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: decrementing and referencing a variable in a complex condition can cause unintended side-effects due to C++'s order of evaluation, consider moving the modification outside of the condition to avoid misunderstandings [bugprone-inc-dec-in-conditions]
+}
+
+bool doubleCheck(Container x) {
+  auto it = x.begin();
+  auto it2 = x.begin();
+  return (--it != x.end() and ++it2 != x.end()) and (*it == *it2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: decrementing and referencing a variable in a complex condition can cause unintended side-effects due to C++'s order of evaluation, consider moving the modification outside of the condition to avoid misundersta

[clang-tools-extra] f27f22b - [clang-tidy] Added bugprone-inc-dec-in-conditions check

2023-07-30 Thread Piotr Zegar via cfe-commits


Author: Piotr Zegar
Date: 2023-07-30T13:19:51Z
New Revision: f27f22b34516dcb744077710c19d475367e3ef03

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

LOG: [clang-tidy] Added bugprone-inc-dec-in-conditions check

Detects when a variable is both incremented/decremented and referenced inside a
complex condition and suggests moving them outside to avoid ambiguity in the
variable's value.

Reviewed By: xgupta

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

Added: 
clang-tools-extra/clang-tidy/bugprone/IncDecInConditionsCheck.cpp
clang-tools-extra/clang-tidy/bugprone/IncDecInConditionsCheck.h
clang-tools-extra/clang-tidy/utils/Matchers.cpp
clang-tools-extra/docs/clang-tidy/checks/bugprone/inc-dec-in-conditions.rst

clang-tools-extra/test/clang-tidy/checkers/bugprone/inc-dec-in-conditions.cpp

Modified: 
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/utils/CMakeLists.txt
clang-tools-extra/clang-tidy/utils/Matchers.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index ceb966c655d3a8..853c6d51458793 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -27,6 +27,7 @@
 #include "ForwardingReferenceOverloadCheck.h"
 #include "ImplicitWideningOfMultiplicationResultCheck.h"
 #include "InaccurateEraseCheck.h"
+#include "IncDecInConditionsCheck.h"
 #include "IncorrectRoundingsCheck.h"
 #include "InfiniteLoopCheck.h"
 #include "IntegerDivisionCheck.h"
@@ -122,6 +123,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-inaccurate-erase");
 CheckFactories.registerCheck(
 "bugprone-switch-missing-default-case");
+CheckFactories.registerCheck(
+"bugprone-inc-dec-in-conditions");
 CheckFactories.registerCheck(
 "bugprone-incorrect-roundings");
 CheckFactories.registerCheck("bugprone-infinite-loop");

diff  --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 4fa32aa61a4b09..5669d6bf4205f6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -23,6 +23,7 @@ add_clang_library(clangTidyBugproneModule
   ImplicitWideningOfMultiplicationResultCheck.cpp
   InaccurateEraseCheck.cpp
   SwitchMissingDefaultCaseCheck.cpp
+  IncDecInConditionsCheck.cpp
   IncorrectRoundingsCheck.cpp
   InfiniteLoopCheck.cpp
   IntegerDivisionCheck.cpp

diff  --git a/clang-tools-extra/clang-tidy/bugprone/IncDecInConditionsCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/IncDecInConditionsCheck.cpp
new file mode 100644
index 00..16f43128d55e87
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/IncDecInConditionsCheck.cpp
@@ -0,0 +1,82 @@
+//===--- IncDecInConditionsCheck.cpp - clang-tidy 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "IncDecInConditionsCheck.h"
+#include "../utils/Matchers.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+AST_MATCHER(BinaryOperator, isLogicalOperator) { return Node.isLogicalOp(); }
+
+AST_MATCHER(UnaryOperator, isUnaryPrePostOperator) {
+  return Node.isPrefix() || Node.isPostfix();
+}
+
+AST_MATCHER(CXXOperatorCallExpr, isPrePostOperator) {
+  return Node.getOperator() == OO_PlusPlus ||
+ Node.getOperator() == OO_MinusMinus;
+}
+
+void IncDecInConditionsCheck::registerMatchers(MatchFinder *Finder) {
+  auto OperatorMatcher = expr(
+  anyOf(binaryOperator(anyOf(isComparisonOperator(), isLogicalOperator())),
+cxxOperatorCallExpr(isComparisonOperator(;
+
+  Finder->addMatcher(
+  expr(
+  OperatorMatcher, unless(isExpansionInSystemHeader()),
+  unless(hasAncestor(OperatorMatcher)), expr().bind("parent"),
+
+  forEachDescendant(
+  expr(anyOf(unaryOperator(isUnaryPrePostOperator(),
+   
hasUnaryOperand(expr().bind("operand"))),
+ cxxOperatorCallExpr(
+ isPrePostOperator(),
+

[PATCH] D156274: [NFC] Avoid potential dereferencing of nullptr.

2023-07-30 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM! Are you planning to work on the ObjC issues separately? (Not suggesting 
you have to! But if you don't intend to, can you file an issue in GitHub so we 
don't lose track of the problem?)


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

https://reviews.llvm.org/D156274

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

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

2023-07-30 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D156363#4544782 , @thakis wrote:

> http://45.33.8.238/linux/113921/step_7.txt is failing due to this. Is this 
> something on my end?

Nope, this same failure is happening in our build farm:
https://lab.llvm.org/buildbot/#/builders/197/builds/8677
https://lab.llvm.org/buildbot/#/builders/179/builds/6872
https://lab.llvm.org/buildbot/#/builders/198/builds/4171

(and others)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156363

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

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

2023-07-30 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

http://45.33.8.238/linux/113921/step_7.txt is failing due to this. Is this 
something on my end?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156363

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

[PATCH] D156608: [clang-tidy] Do not warn on macros starting with underscore and lowercase letter in bugprone-reserved-identifier

2023-07-30 Thread Carlos Galvez via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2992d084774f: [clang-tidy] Do not warn on macros starting 
with underscore and lowercase… (authored by carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156608

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
@@ -45,7 +45,7 @@
 
 #define __macro(m) int m = 0
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '__macro', which is a reserved identifier [bugprone-reserved-identifier]
-// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
+// CHECK-FIXES: {{^}}#define _macro(m) int m = 0{{$}}
 
 namespace __ns {
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses identifier '__ns', which is a reserved identifier [bugprone-reserved-identifier]
@@ -115,9 +115,9 @@
 
 //
 
+// OK, this rule does not apply to macros:
+// https://github.com/llvm/llvm-project/issues/64130#issuecomment-1655751676
 #define _macro(m) int m = 0
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '_macro', which is reserved in the global namespace [bugprone-reserved-identifier]
-// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
 
 namespace _ns {
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses identifier '_ns', which is reserved in the global namespace [bugprone-reserved-identifier]
@@ -171,10 +171,9 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses identifier '_', which is reserved in the global namespace; cannot be fixed automatically [bugprone-reserved-identifier]
 // CHECK-FIXES: {{^}}int _;{{$}}
 
+// This should not trigger a warning
 // https://github.com/llvm/llvm-project/issues/52895
 #define _5_kmph_rpm 459
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '_5_kmph_rpm', which is reserved in the global namespace; cannot be fixed automatically [bugprone-reserved-identifier]
-// CHECK-FIXES: {{^}}#define _5_kmph_rpm 459{{$}}
 
 // these should pass
 #define MACRO(m) int m = 0
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -140,6 +140,10 @@
 Changes in existing checks
 ^^
 
+- Fixed bug in :doc:`bugprone-reserved-identifier
+  `, so that it does not warn
+  on macros starting with underscore and lowercase letter.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
@@ -102,13 +102,15 @@
 }
 
 static bool startsWithUnderscoreInGlobalNamespace(StringRef Name,
-  bool IsInGlobalNamespace) {
-  return IsInGlobalNamespace && Name.size() >= 1 && Name[0] == '_';
+  bool IsInGlobalNamespace,
+  bool IsMacro) {
+  return !IsMacro && IsInGlobalNamespace && Name.size() >= 1 && Name[0] == '_';
 }
 
 static std::optional
-getUnderscoreGlobalNamespaceFixup(StringRef Name, bool IsInGlobalNamespace) {
-  if (startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace))
+getUnderscoreGlobalNamespaceFixup(StringRef Name, bool IsInGlobalNamespace,
+  bool IsMacro) {
+  if (startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace, IsMacro))
 return std::string(Name.drop_front(1));
   return std::nullopt;
 }
@@ -123,7 +125,7 @@
 }
 
 static std::optional
-getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace,
+getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace, bool IsMacro,
const LangOptions &LangOpts, bool Invert,
ArrayRef AllowedIdentifiers) {
   assert(!Name.empty());
@@ -158,15 +160,16 @@
   AppendFailure(DoubleUnderscoreTag, std::move(*Fixup));
 if (auto Fixup = getUnderscoreCapitalFixup(InProgressFixup()))
   AppendFailure(UnderscoreCapitalTag, std::move(*Fixup));
-if (auto Fixup = getUnderscoreGlobalNamespaceFixup(InProgressFixup(),
-   IsInGlobalNamespace))
+if (auto Fixup = getUnderscoreGlobalNamespaceFixup(
+InProgressFixup(), IsInGlo

[clang-tools-extra] 2992d08 - [clang-tidy] Do not warn on macros starting with underscore and lowercase letter in bugprone-reserved-identifier

2023-07-30 Thread Carlos Galvez via cfe-commits


Author: Carlos Galvez
Date: 2023-07-30T12:10:48Z
New Revision: 2992d084774f44e7626a7d640fe6c30163db450e

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

LOG: [clang-tidy] Do not warn on macros starting with underscore and lowercase 
letter in bugprone-reserved-identifier

Fixes #64130

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
index aaf10f63fb070d..fb04e6e0fa9361 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
@@ -102,13 +102,15 @@ static std::optional 
getUnderscoreCapitalFixup(StringRef Name) {
 }
 
 static bool startsWithUnderscoreInGlobalNamespace(StringRef Name,
-  bool IsInGlobalNamespace) {
-  return IsInGlobalNamespace && Name.size() >= 1 && Name[0] == '_';
+  bool IsInGlobalNamespace,
+  bool IsMacro) {
+  return !IsMacro && IsInGlobalNamespace && Name.size() >= 1 && Name[0] == '_';
 }
 
 static std::optional
-getUnderscoreGlobalNamespaceFixup(StringRef Name, bool IsInGlobalNamespace) {
-  if (startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace))
+getUnderscoreGlobalNamespaceFixup(StringRef Name, bool IsInGlobalNamespace,
+  bool IsMacro) {
+  if (startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace, 
IsMacro))
 return std::string(Name.drop_front(1));
   return std::nullopt;
 }
@@ -123,7 +125,7 @@ static std::string getNonReservedFixup(std::string Name) {
 }
 
 static std::optional
-getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace,
+getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace, bool IsMacro,
const LangOptions &LangOpts, bool Invert,
ArrayRef AllowedIdentifiers) {
   assert(!Name.empty());
@@ -158,15 +160,16 @@ getFailureInfoImpl(StringRef Name, bool 
IsInGlobalNamespace,
   AppendFailure(DoubleUnderscoreTag, std::move(*Fixup));
 if (auto Fixup = getUnderscoreCapitalFixup(InProgressFixup()))
   AppendFailure(UnderscoreCapitalTag, std::move(*Fixup));
-if (auto Fixup = getUnderscoreGlobalNamespaceFixup(InProgressFixup(),
-   IsInGlobalNamespace))
+if (auto Fixup = getUnderscoreGlobalNamespaceFixup(
+InProgressFixup(), IsInGlobalNamespace, IsMacro))
   AppendFailure(GlobalUnderscoreTag, std::move(*Fixup));
 
 return Info;
   }
   if (!(hasReservedDoubleUnderscore(Name, LangOpts) ||
 startsWithUnderscoreCapital(Name) ||
-startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace)))
+startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace,
+  IsMacro)))
 return FailureInfo{NonReservedTag, getNonReservedFixup(std::string(Name))};
   return std::nullopt;
 }
@@ -177,16 +180,17 @@ ReservedIdentifierCheck::getDeclFailureInfo(const 
NamedDecl *Decl,
   assert(Decl && Decl->getIdentifier() && !Decl->getName().empty() &&
  !Decl->isImplicit() &&
  "Decl must be an explicit identifier with a name.");
-  return getFailureInfoImpl(Decl->getName(),
-isa(Decl->getDeclContext()),
-getLangOpts(), Invert, AllowedIdentifiers);
+  return getFailureInfoImpl(
+  Decl->getName(), isa(Decl->getDeclContext()),
+  /*IsMacro = */ false, getLangOpts(), Invert, AllowedIdentifiers);
 }
 
 std::optional
 ReservedIdentifierCheck::getMacroFailureInfo(const Token &MacroNameTok,
  const SourceManager &) const {
   return getFailureInfoImpl(MacroNameTok.getIdentifierInfo()->getName(), true,
-getLangOpts(), Invert, AllowedIdentifiers);
+/*IsMacro = */ true, getLangOpts(), Invert,
+AllowedIdentifiers);
 }
 
 RenamerClangTidyCheck::DiagInfo

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index a93ff58c60e2c0..000b80aadc0237 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -140,6 +140,10 @@ New check aliases
 Changes in existing checks
 ^^
 
+- Fixed bug in :doc

[PATCH] D156608: [clang-tidy] Do not warn on macros starting with underscore and lowercase letter in bugprone-reserved-identifier

2023-07-30 Thread Piotr Zegar via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156608

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

[PATCH] D156608: [clang-tidy] Do not warn on macros starting with underscore and lowercase letter in bugprone-reserved-identifier

2023-07-30 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp updated this revision to Diff 545415.
carlosgalvezp added a comment.

Remove extra newline


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156608

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
@@ -45,7 +45,7 @@
 
 #define __macro(m) int m = 0
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '__macro', which is a reserved identifier [bugprone-reserved-identifier]
-// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
+// CHECK-FIXES: {{^}}#define _macro(m) int m = 0{{$}}
 
 namespace __ns {
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses identifier '__ns', which is a reserved identifier [bugprone-reserved-identifier]
@@ -115,9 +115,9 @@
 
 //
 
+// OK, this rule does not apply to macros:
+// https://github.com/llvm/llvm-project/issues/64130#issuecomment-1655751676
 #define _macro(m) int m = 0
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '_macro', which is reserved in the global namespace [bugprone-reserved-identifier]
-// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
 
 namespace _ns {
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses identifier '_ns', which is reserved in the global namespace [bugprone-reserved-identifier]
@@ -171,10 +171,9 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses identifier '_', which is reserved in the global namespace; cannot be fixed automatically [bugprone-reserved-identifier]
 // CHECK-FIXES: {{^}}int _;{{$}}
 
+// This should not trigger a warning
 // https://github.com/llvm/llvm-project/issues/52895
 #define _5_kmph_rpm 459
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '_5_kmph_rpm', which is reserved in the global namespace; cannot be fixed automatically [bugprone-reserved-identifier]
-// CHECK-FIXES: {{^}}#define _5_kmph_rpm 459{{$}}
 
 // these should pass
 #define MACRO(m) int m = 0
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -140,6 +140,10 @@
 Changes in existing checks
 ^^
 
+- Fixed bug in :doc:`bugprone-reserved-identifier
+  `, so that it does not warn
+  on macros starting with underscore and lowercase letter.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
@@ -102,13 +102,15 @@
 }
 
 static bool startsWithUnderscoreInGlobalNamespace(StringRef Name,
-  bool IsInGlobalNamespace) {
-  return IsInGlobalNamespace && Name.size() >= 1 && Name[0] == '_';
+  bool IsInGlobalNamespace,
+  bool IsMacro) {
+  return !IsMacro && IsInGlobalNamespace && Name.size() >= 1 && Name[0] == '_';
 }
 
 static std::optional
-getUnderscoreGlobalNamespaceFixup(StringRef Name, bool IsInGlobalNamespace) {
-  if (startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace))
+getUnderscoreGlobalNamespaceFixup(StringRef Name, bool IsInGlobalNamespace,
+  bool IsMacro) {
+  if (startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace, IsMacro))
 return std::string(Name.drop_front(1));
   return std::nullopt;
 }
@@ -123,7 +125,7 @@
 }
 
 static std::optional
-getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace,
+getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace, bool IsMacro,
const LangOptions &LangOpts, bool Invert,
ArrayRef AllowedIdentifiers) {
   assert(!Name.empty());
@@ -158,15 +160,16 @@
   AppendFailure(DoubleUnderscoreTag, std::move(*Fixup));
 if (auto Fixup = getUnderscoreCapitalFixup(InProgressFixup()))
   AppendFailure(UnderscoreCapitalTag, std::move(*Fixup));
-if (auto Fixup = getUnderscoreGlobalNamespaceFixup(InProgressFixup(),
-   IsInGlobalNamespace))
+if (auto Fixup = getUnderscoreGlobalNamespaceFixup(
+InProgressFixup(), IsInGlobalNamespace, IsMacro))
   AppendFailure(GlobalUnderscoreTag, std::move(*Fixup));
 
 return Info;

[PATCH] D156608: [clang-tidy] Do not warn on macros starting with underscore and lowercase letter in bugprone-reserved-identifier

2023-07-30 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp created this revision.
Herald added subscribers: PiotrZSL, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Fixes #64130


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156608

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
@@ -45,7 +45,7 @@
 
 #define __macro(m) int m = 0
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '__macro', which is a reserved identifier [bugprone-reserved-identifier]
-// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
+// CHECK-FIXES: {{^}}#define _macro(m) int m = 0{{$}}
 
 namespace __ns {
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses identifier '__ns', which is a reserved identifier [bugprone-reserved-identifier]
@@ -115,9 +115,9 @@
 
 //
 
+// OK, this rule does not apply to macros:
+// https://github.com/llvm/llvm-project/issues/64130#issuecomment-1655751676
 #define _macro(m) int m = 0
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '_macro', which is reserved in the global namespace [bugprone-reserved-identifier]
-// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
 
 namespace _ns {
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses identifier '_ns', which is reserved in the global namespace [bugprone-reserved-identifier]
@@ -171,10 +171,9 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses identifier '_', which is reserved in the global namespace; cannot be fixed automatically [bugprone-reserved-identifier]
 // CHECK-FIXES: {{^}}int _;{{$}}
 
+// This should not trigger a warning
 // https://github.com/llvm/llvm-project/issues/52895
 #define _5_kmph_rpm 459
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '_5_kmph_rpm', which is reserved in the global namespace; cannot be fixed automatically [bugprone-reserved-identifier]
-// CHECK-FIXES: {{^}}#define _5_kmph_rpm 459{{$}}
 
 // these should pass
 #define MACRO(m) int m = 0
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -140,6 +140,10 @@
 Changes in existing checks
 ^^
 
+- Fixed bug in :doc:`bugprone-reserved-identifier
+  `, so that it does not warn
+  on macros starting with underscore and lowercase letter.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
@@ -102,13 +102,15 @@
 }
 
 static bool startsWithUnderscoreInGlobalNamespace(StringRef Name,
-  bool IsInGlobalNamespace) {
-  return IsInGlobalNamespace && Name.size() >= 1 && Name[0] == '_';
+  bool IsInGlobalNamespace,
+  bool IsMacro) {
+  return !IsMacro && IsInGlobalNamespace && Name.size() >= 1 && Name[0] == '_';
 }
 
 static std::optional
-getUnderscoreGlobalNamespaceFixup(StringRef Name, bool IsInGlobalNamespace) {
-  if (startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace))
+getUnderscoreGlobalNamespaceFixup(StringRef Name, bool IsInGlobalNamespace,
+  bool IsMacro) {
+  if (startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace, IsMacro))
 return std::string(Name.drop_front(1));
   return std::nullopt;
 }
@@ -123,7 +125,7 @@
 }
 
 static std::optional
-getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace,
+getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace, bool IsMacro,
const LangOptions &LangOpts, bool Invert,
ArrayRef AllowedIdentifiers) {
   assert(!Name.empty());
@@ -158,15 +160,17 @@
   AppendFailure(DoubleUnderscoreTag, std::move(*Fixup));
 if (auto Fixup = getUnderscoreCapitalFixup(InProgressFixup()))
   AppendFailure(UnderscoreCapitalTag, std::move(*Fixup));
-if (auto Fixup = getUnderscoreGlobalNamespaceFixup(InProgressFixup(),
-   IsInGlobalNamespace))
+if (auto Fixup = getUnderscoreGlobalNamespaceFixup(
+InProgress

[PATCH] D154576: [RISCV] RISCV vector calling convention (1/2)

2023-07-30 Thread Brandon Wu via Phabricator via cfe-commits

4vtomat updated this revision to Diff 545413.
4vtomat marked 2 inline comments as done.
4vtomat added a comment.

Resolved Aaron's comments.
Also group some code and refactoring some code to make it much clear.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154576

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/RISCV/riscv-vector-callingconv-llvm-ir.c
  clang/test/CodeGen/RISCV/riscv-vector-callingconv.cpp
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/RISCV/RISCVCallingConv.td
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
  llvm/test/CodeGen/RISCV/rvv/callee-saved-regs.ll

Index: llvm/test/CodeGen/RISCV/rvv/callee-saved-regs.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rvv/callee-saved-regs.ll
@@ -0,0 +1,221 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+m -mattr=+v -O2 < %s \
+; RUN:| FileCheck --check-prefix=SPILL-O2 %s
+
+define  @test_vector_std( %va) nounwind {
+; SPILL-O2-LABEL: test_vector_std:
+; SPILL-O2:   # %bb.0: # %entry
+; SPILL-O2-NEXT:addi sp, sp, -16
+; SPILL-O2-NEXT:csrr a0, vlenb
+; SPILL-O2-NEXT:slli a0, a0, 1
+; SPILL-O2-NEXT:sub sp, sp, a0
+; SPILL-O2-NEXT:addi a0, sp, 16
+; SPILL-O2-NEXT:vs1r.v v8, (a0) # Unknown-size Folded Spill
+; SPILL-O2-NEXT:#APP
+; SPILL-O2-NEXT:#NO_APP
+; SPILL-O2-NEXT:vl1r.v v8, (a0) # Unknown-size Folded Reload
+; SPILL-O2-NEXT:csrr a0, vlenb
+; SPILL-O2-NEXT:slli a0, a0, 1
+; SPILL-O2-NEXT:add sp, sp, a0
+; SPILL-O2-NEXT:addi sp, sp, 16
+; SPILL-O2-NEXT:ret
+entry:
+  call void asm sideeffect "",
+  "~{v0},~{v1},~{v2},~{v3},~{v4},~{v5},~{v6},~{v7},~{v8},~{v9},~{v10},~{v11},~{v12},~{v13},~{v14},~{v15},~{v16},~{v17},~{v18},~{v19},~{v20},~{v21},~{v22},~{v23},~{v24},~{v25},~{v26},~{v27},~{v28},~{v29},~{v30},~{v31}"()
+
+  ret  %va
+}
+
+define riscv_vector_cc  @test_vector_callee( %va) nounwind {
+; SPILL-O2-LABEL: test_vector_callee:
+; SPILL-O2:   # %bb.0: # %entry
+; SPILL-O2-NEXT:addi sp, sp, -16
+; SPILL-O2-NEXT:csrr a0, vlenb
+; SPILL-O2-NEXT:slli a0, a0, 4
+; SPILL-O2-NEXT:sub sp, sp, a0
+; SPILL-O2-NEXT:csrr a0, vlenb
+; SPILL-O2-NEXT:slli a1, a0, 4
+; SPILL-O2-NEXT:sub a0, a1, a0
+; SPILL-O2-NEXT:add a0, sp, a0
+; SPILL-O2-NEXT:addi a0, a0, 16
+; SPILL-O2-NEXT:vs1r.v v1, (a0) # Unknown-size Folded Spill
+; SPILL-O2-NEXT:csrr a0, vlenb
+; SPILL-O2-NEXT:li a1, 14
+; SPILL-O2-NEXT:mul a0, a0, a1
+; SPILL-O2-NEXT:add a0, sp, a0
+; SPILL-O2-NEXT:addi a0, a0, 16
+; SPILL-O2-NEXT:vs1r.v v2, (a0) # Unknown-size Folded Spill
+; SPILL-O2-NEXT:csrr a0, vlenb
+; SPILL-O2-NEXT:li a1, 13
+; SPILL-O2-NEXT:mul a0, a0, a1
+; SPILL-O2-NEXT:add a0, sp, a0
+; SPILL-O2-NEXT:addi a0, a0, 16
+; SPILL-O2-NEXT:vs1r.v v3, (a0) # Unknown-size Folded Spill
+; SPILL-O2-NEXT:csrr a0, vlenb
+; SPILL-O2-NEXT:li a1, 12
+; SPILL-O2-NEXT:mul a0, a0, a1
+; SPILL-O2-NEXT:add a0, sp, a0
+; SPILL-O2-NEXT:addi a0, a0, 16
+; SPILL-O2-NEXT:vs1r.v v4, (a0) # Unknown-size Folded Spill
+; SPILL-O2-NEXT:csrr a0, vlenb
+; SPILL-O2-NEXT:li a1, 11
+; SPILL-O2-NEXT:mul a0, a0, a1
+; SPILL-O2-NEXT:add a0, sp, a0
+; SPILL-O2-NEXT:addi a0, a0, 16
+; SPILL-O2-NEXT:vs1r.v v5, (a0) # Unknown-size Folded Spill
+; SPILL-O2-NEXT:csrr a0, vlenb
+; SPILL-O2-NEXT:li a1, 10
+; SPILL-O2-NEXT:mul a0, a0, a1
+; SPILL-O2-NEXT:add a0, sp, a0
+; SPILL-O2-NEXT:addi a0, a0, 16
+; SPILL-O2-NEXT:vs1r.v v6, (a0) # Unknown-size Folded Spill
+; SPILL-O2-NEXT:csrr a0, vlenb
+; SPILL-O2-NEXT:slli a1, a0, 3
+; SPILL-O2-NEXT:add a0, a1, a0
+; SPILL-O2-NEXT:add a0, sp, a0
+; SPILL-O2-NEXT:addi a0, a0, 16
+; SPILL-O2-NEXT:vs1r.v v7, (a0) # Unknown-size Folded Spill
+; SPILL-O2-NEXT:csrr a0, vlenb
+; SPILL-O2-NEXT:slli a0, a0, 3
+; SPILL-O2-NEXT:add a0, sp, a0
+; SPILL-O2-NEXT:addi a0, a0, 16
+; SPILL-O2-NEXT:vs1r.v v24, (a0) # Unknown-size Folded Spill
+; SPILL-O2-NEXT:csrr a0, vlenb
+; SPILL-O2-NEXT:slli a1, a0, 3
+; SPILL-O2-NEXT:sub a0, a1, a0
+; SPILL-O2-NEXT:add a0, sp, a0
+; SPILL-O2-NEXT:addi a0, a0, 16
+; SPILL-O2-NEXT:vs1r.v v25,

[PATCH] D146054: [RISCV] Add --print-supported-extensions support

2023-07-30 Thread Brandon Wu via Phabricator via cfe-commits

4vtomat updated this revision to Diff 545412.
4vtomat added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/print-supported-extensions.c
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/RISCVISAInfo.cpp

Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -208,6 +208,29 @@
 #endif
 }
 
+void llvm::riscvExtensionsHelp() {
+  outs() << "All available -march extensions for RISC-V\n\n";
+  outs() << '\t' << left_justify("Name", 20) << "Version\n";
+
+  RISCVISAInfo::OrderedExtensionMap ExtMap;
+  for (const auto &E : SupportedExtensions)
+ExtMap[E.Name] = { E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+outs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  outs() << "\nExperimental extensions\n";
+  ExtMap.clear();
+  for (const auto &E : SupportedExperimentalExtensions)
+ExtMap[E.Name] = { E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+outs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  outs() << "\nUse -march to specify the target's extension.\n"
+"For example, clang -march=rv32i_v1p0\n";
+}
+
 static bool stripExperimentalPrefix(StringRef &Ext) {
   return Ext.consume_front("experimental-");
 }
Index: llvm/include/llvm/Support/RISCVISAInfo.h
===
--- llvm/include/llvm/Support/RISCVISAInfo.h
+++ llvm/include/llvm/Support/RISCVISAInfo.h
@@ -22,6 +22,8 @@
   unsigned MinorVersion;
 };
 
+void riscvExtensionsHelp();
+
 class RISCVISAInfo {
 public:
   RISCVISAInfo(const RISCVISAInfo &) = delete;
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/RISCVISAInfo.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -182,6 +183,12 @@
   return 0;
 }
 
+/// Print supported extensions of the RISCV target.
+static void printSupportedExtensions() {
+  llvm::riscvExtensionsHelp();
+}
+
+
 int cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr) {
   ensureSufficientStack();
 
@@ -221,6 +228,10 @@
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
 
+  // --print-supported-extensions takes priority over the actual compilation.
+  if (Clang->getFrontendOpts().PrintSupportedExtensions)
+return printSupportedExtensions(), 0;
+
   // Infer the builtin include path if unspecified.
   if (Clang->getHeaderSearchOpts().UseBuiltinIncludes &&
   Clang->getHeaderSearchOpts().ResourceDir.empty())
Index: clang/test/Driver/print-supported-extensions.c
===
--- /dev/null
+++ clang/test/Driver/print-supported-extensions.c
@@ -0,0 +1,122 @@
+/// Test that --print-supported-extensions lists supported extensions.
+
+// REQUIRES: riscv-registered-target
+
+// RUN: %clang --target=riscv64 --print-supported-extensions 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=warning: --strict-whitespace --match-full-lines %s
+
+//  CHECK:Target: riscv64
+//  CHECK:All available -march extensions for RISC-V
+//  CHECK:	NameVersion
+// CHECK-NEXT:	i   2.1
+// CHECK-NEXT:	e   2.0
+// CHECK-NEXT:	m   2.0
+// CHECK-NEXT:	a   2.1
+// CHECK-NEXT:	f   2.2
+// CHECK-NEXT:	d   2.2
+// CHECK-NEXT:	c   2.0
+// CHECK-NEXT:	v   1.0
+// CHECK-NEXT:	h   1.0
+// CHECK-NEXT:	zicbom  1.0
+// CHECK-NEXT:	zicbop  1.0
+// CHECK-NEXT:	zicboz  1.0
+// CHECK-NEXT:	zicntr  1.0
+// CHECK-NEXT:	zicsr   2.0
+// CHECK-NEXT:	zifencei2.0
+// CHECK-NEXT:	zihintpause 2.0
+// CHECK-NEXT:	zihpm   1.0
+// CHECK-NEXT:	zmmul   1.0
+// CHECK-NEXT:	zawrs   1.0
+// CHECK-NEXT:	zfh 1.0
+// CHECK-NEXT:	zfhmin  1.0
+// CHECK-NEXT:	zfinx   1.0
+// CHECK-NEXT:	zdinx   1.0
+// CHECK-NEXT:	zca 1.0
+// CHECK-NEXT:	zcb 1.0
+// CHECK-NEXT:	zcd

[PATCH] D146054: [RISCV] Add --print-supported-extensions support

2023-07-30 Thread Brandon Wu via Phabricator via cfe-commits

4vtomat updated this revision to Diff 545411.
4vtomat marked 6 inline comments as done.
4vtomat added a comment.

Resolved MaskRay's comments, thanks for reviewing!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/print-supported-extensions.c
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/RISCVISAInfo.cpp

Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -198,6 +198,29 @@
 #endif
 }
 
+void llvm::riscvExtensionsHelp() {
+  outs() << "All available -march extensions for RISC-V\n\n";
+  outs() << '\t' << left_justify("Name", 20) << "Version\n";
+
+  RISCVISAInfo::OrderedExtensionMap ExtMap;
+  for (const auto &E : SupportedExtensions)
+ExtMap[E.Name] = { E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+outs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  outs() << "\nExperimental extensions\n";
+  ExtMap.clear();
+  for (const auto &E : SupportedExperimentalExtensions)
+ExtMap[E.Name] = { E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+outs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  outs() << "\nUse -march to specify the target's extension.\n"
+"For example, clang -march=rv32i_v1p0\n";
+}
+
 static bool stripExperimentalPrefix(StringRef &Ext) {
   return Ext.consume_front("experimental-");
 }
Index: llvm/include/llvm/Support/RISCVISAInfo.h
===
--- llvm/include/llvm/Support/RISCVISAInfo.h
+++ llvm/include/llvm/Support/RISCVISAInfo.h
@@ -22,6 +22,8 @@
   unsigned MinorVersion;
 };
 
+void riscvExtensionsHelp();
+
 class RISCVISAInfo {
 public:
   RISCVISAInfo(const RISCVISAInfo &) = delete;
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/RISCVISAInfo.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -182,6 +183,12 @@
   return 0;
 }
 
+/// Print supported extensions of the RISCV target.
+static void printSupportedExtensions() {
+  llvm::riscvExtensionsHelp();
+}
+
+
 int cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr) {
   ensureSufficientStack();
 
@@ -221,6 +228,10 @@
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
 
+  // --print-supported-extensions takes priority over the actual compilation.
+  if (Clang->getFrontendOpts().PrintSupportedExtensions)
+return printSupportedExtensions(), 0;
+
   // Infer the builtin include path if unspecified.
   if (Clang->getHeaderSearchOpts().UseBuiltinIncludes &&
   Clang->getHeaderSearchOpts().ResourceDir.empty())
Index: clang/test/Driver/print-supported-extensions.c
===
--- /dev/null
+++ clang/test/Driver/print-supported-extensions.c
@@ -0,0 +1,122 @@
+/// Test that --print-supported-extensions lists supported extensions.
+
+// REQUIRES: riscv-registered-target
+
+// RUN: %clang --target=riscv64 --print-supported-extensions 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=warning: --strict-whitespace --match-full-lines %s
+
+//  CHECK:Target: riscv64
+//  CHECK:All available -march extensions for RISC-V
+//  CHECK:	NameVersion
+// CHECK-NEXT:	i   2.1
+// CHECK-NEXT:	e   2.0
+// CHECK-NEXT:	m   2.0
+// CHECK-NEXT:	a   2.1
+// CHECK-NEXT:	f   2.2
+// CHECK-NEXT:	d   2.2
+// CHECK-NEXT:	c   2.0
+// CHECK-NEXT:	v   1.0
+// CHECK-NEXT:	h   1.0
+// CHECK-NEXT:	zicbom  1.0
+// CHECK-NEXT:	zicbop  1.0
+// CHECK-NEXT:	zicboz  1.0
+// CHECK-NEXT:	zicntr  1.0
+// CHECK-NEXT:	zicsr   2.0
+// CHECK-NEXT:	zifencei2.0
+// CHECK-NEXT:	zihintpause 2.0
+// CHECK-NEXT:	zihpm   1.0
+// CHECK-NEXT:	zmmul   1.0
+// CHECK-NEXT:	zawrs   1.0
+// CHECK-NEXT:	zfh 1.0
+// CHECK-NEXT:	zfhmin  1.0
+// CHECK-NEXT:	zfinx   1.0
+// CHECK-NEXT:	zdinx   1.0
+// CHECK-NEXT:	zca

[PATCH] D154576: [RISCV] RISCV vector calling convention (1/2)

2023-07-30 Thread Brandon Wu via Phabricator via cfe-commits

4vtomat marked 2 inline comments as done.
4vtomat added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:3267
   case CC_PreserveAll:
+  case CC_RISCVVectorCall:
 // FIXME: we should be mangling all of the above.

aaron.ballman wrote:
> Is it possible to use this calling convention on Windows where we'd hit the 
> Microsoft name mangler?
Thanks for reviewing. However since RISCV target currently only supports `elf`, 
so maybe we will do this in the future!



Comment at: clang/test/CodeGen/RISCV/riscv-vector-cc-attr.c:1
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: riscv-registered-target

aaron.ballman wrote:
> You should also have Sema tests for various things (attribute accepts no 
> arguments, only applies to functions, does/doesn't silently convert to cdecl, 
> etc). You should also have some tests using the attribute as a type attribute 
> instead of a declaration attribute.
> 
> You should also have C++ tests for Sema and codegen for things like putting 
> the calling convention on a member function or a lambda to ensure those do 
> reasonable things with the convention.
Good idea, thanks for your review, I will add some tests for Sema!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154576

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

[PATCH] D154576: [RISCV] RISCV vector calling convention (1/2)

2023-07-30 Thread Brandon Wu via Phabricator via cfe-commits

4vtomat added a comment.

In D154576#4483919 , @lhtin wrote:

> In D154576#4476300 , @4vtomat wrote:
>
>> In D154576#4476291 , @craig.topper 
>> wrote:
>>
>>> Does this only change the calling convention when 
>>> `__attribute__((riscv_vector_cc))` is used? The attribute should be 
>>> mentioned in the patch description.
>>
>> Yes it only changes when `__attribute__((riscv_vector_cc))` is used, thanks 
>> for reminding, I will mention in the description.
>
> In addition to using attributes, functions that use vector registers to pass 
> arguments or return values also need to comply with the new calling 
> convention.

Thanks for pointing this out! However, since it would make a tons of test cases 
change, I will make it in another patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154576

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

[PATCH] D156274: [NFC] Avoid potential dereferencing of nullptr.

2023-07-30 Thread Sindhu Chittireddy via Phabricator via cfe-commits

schittir updated this revision to Diff 545410.
schittir added a comment.

Remove changes in CGObjCMac.cpp


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

https://reviews.llvm.org/D156274

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -6601,7 +6601,7 @@
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   // FIXME: results is always empty, this appears to be dead.
-  if (!Results.empty() && NNS->isDependent())
+  if (!Results.empty() && NNS && NNS->isDependent())
 Results.AddResult("template");
 
   // If the scope is a concept-constrained type parameter, infer nested
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -1656,6 +1656,7 @@
   // Otherwise, use the complete destructor name. This is relevant if a
   // class with a destructor is declared within a destructor.
   mangleCXXDtorType(Dtor_Complete);
+assert(ND);
 writeAbiTags(ND, AdditionalAbiTags);
 break;
 


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -6601,7 +6601,7 @@
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   // FIXME: results is always empty, this appears to be dead.
-  if (!Results.empty() && NNS->isDependent())
+  if (!Results.empty() && NNS && NNS->isDependent())
 Results.AddResult("template");
 
   // If the scope is a concept-constrained type parameter, infer nested
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -1656,6 +1656,7 @@
   // Otherwise, use the complete destructor name. This is relevant if a
   // class with a destructor is declared within a destructor.
   mangleCXXDtorType(Dtor_Complete);
+assert(ND);
 writeAbiTags(ND, AdditionalAbiTags);
 break;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

58 matches

Mail list logo