[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 488497.
ccotter added a comment.

- Does not offer fixits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
@@ -0,0 +1,224 @@
+// RUN: %check_clang_tidy -std=c++14-or-later %s cppcoreguidelines-rvalue-reference-param-not-moved %t
+
+// NOLINTBEGIN
+namespace std {
+template 
+struct remove_reference;
+
+template 
+struct remove_reference {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &> {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &&> {
+  typedef _Tp type;
+};
+
+template 
+constexpr typename std::remove_reference<_Tp>::type &(_Tp &&__t) noexcept {
+  return static_cast::type &&>(__t);
+}
+
+template 
+constexpr _Tp &&
+forward(typename remove_reference<_Tp>::type &__t) noexcept {
+  return static_cast<_Tp &&>(__t);
+}
+
+}
+// NOLINTEND
+
+struct Obj {
+  Obj();
+  Obj(const Obj&);
+  Obj& operator=(const Obj&);
+  Obj(Obj&&);
+  Obj& operator=(Obj&&);
+  void member() const;
+};
+
+void consumes_object(Obj);
+
+void never_moves_param(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  o.member();
+}
+
+void copies_object(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  Obj copy = o;
+}
+
+template 
+void never_moves_param_template(Obj&& o, T t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  o.member();
+}
+
+void never_moves_params(Obj&& o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  // CHECK-MESSAGES: :[[@LINE-2]]:35: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void never_moves_some_params(Obj&& o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+  Obj other{std::move(o2)};
+}
+
+void never_moves_mixed(Obj o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void lambda_captures_parameter_as_value(Obj&& o) {
+  auto f = [o]() {
+consumes_object(std::move(o));
+  };
+  // CHECK-MESSAGES: :[[@LINE-4]]:41: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void lambda_captures_parameter_as_value_nested(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  auto f = []() {
+auto f_nested = [o]() {
+  consumes_object(std::move(o));
+};
+  };
+  auto f2 = [o]() {
+auto f_nested = []() {
+  consumes_object(std::move(o));
+};
+  };
+  auto f3 = [o]() {
+auto f_nested = []() {
+  auto f_nested_inner = []() {
+consumes_object(std::move(o));
+  };
+};
+  };
+  auto f4 = []() {
+auto f_nested = []() {
+  auto f_nested_inner = [o]() {
+consumes_object(std::move(o));
+  };
+};
+  };
+}
+
+void misc_lambda_checks() {
+  auto never_moves = [](Obj&& o1) {
+Obj other{o1};
+  };
+  // CHECK-MESSAGES: :[[@LINE-3]]:25: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+  auto never_moves_with_auto_param = [](Obj&& o1, auto& v) {
+Obj other{o1};
+  };
+  // 

[PATCH] D141570: [clang] Add no-argument dump to DynTypedNode

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.

I usually use `dump()` when authoring/debugging clang-tidy or similar checks. I 
don't always have ASTContext handy when working with a DynTypedNode (without 
temporarily changing my call stack to pass the ASTContext around for the 
purposes of `dump`).

I put up this as a way of asking whether this kind of change would be 
considered (just as easy to code this up as ask in Discord etc). Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141570

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

[PATCH] D141570: [clang] Add no-argument dump to DynTypedNode

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 488495.
ccotter added a comment.

remove accidental change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141570

Files:
  clang/include/clang/AST/ASTTypeTraits.h
  clang/lib/AST/ASTTypeTraits.cpp


Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ clang/lib/AST/ASTTypeTraits.cpp
@@ -213,6 +213,18 @@
 OS << "Unable to dump values of type " << NodeKind.asStringRef() << "\n";
 }
 
+void DynTypedNode::dump() const {
+  if (const Decl *D = get())
+D->dump();
+  else if (const Stmt *S = get())
+S->dump();
+  else if (const Type *T = get())
+T->dump();
+  else
+llvm::errs() << "Unable to dump values of type " << NodeKind.asStringRef()
+ << "\n";
+}
+
 SourceRange DynTypedNode::getSourceRange() const {
   if (const CXXCtorInitializer *CCI = get())
 return CCI->getSourceRange();
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -296,6 +296,9 @@
   /// Dumps the node to the given output stream.
   void dump(llvm::raw_ostream , const ASTContext ) const;
 
+  /// Dumps the node to \c llvm::errs().
+  void dump() const;
+
   /// For nodes which represent textual entities in the source code,
   /// return their SourceRange.  For all other nodes, return SourceRange().
   SourceRange getSourceRange() const;


Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ clang/lib/AST/ASTTypeTraits.cpp
@@ -213,6 +213,18 @@
 OS << "Unable to dump values of type " << NodeKind.asStringRef() << "\n";
 }
 
+void DynTypedNode::dump() const {
+  if (const Decl *D = get())
+D->dump();
+  else if (const Stmt *S = get())
+S->dump();
+  else if (const Type *T = get())
+T->dump();
+  else
+llvm::errs() << "Unable to dump values of type " << NodeKind.asStringRef()
+ << "\n";
+}
+
 SourceRange DynTypedNode::getSourceRange() const {
   if (const CXXCtorInitializer *CCI = get())
 return CCI->getSourceRange();
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -296,6 +296,9 @@
   /// Dumps the node to the given output stream.
   void dump(llvm::raw_ostream , const ASTContext ) const;
 
+  /// Dumps the node to \c llvm::errs().
+  void dump() const;
+
   /// For nodes which represent textual entities in the source code,
   /// return their SourceRange.  For all other nodes, return SourceRange().
   SourceRange getSourceRange() const;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141570: [clang] Add no-argument dump to DynTypedNode

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 488494.
ccotter added a comment.

format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141570

Files:
  clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
  clang/include/clang/AST/ASTTypeTraits.h
  clang/lib/AST/ASTTypeTraits.cpp


Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ clang/lib/AST/ASTTypeTraits.cpp
@@ -213,6 +213,18 @@
 OS << "Unable to dump values of type " << NodeKind.asStringRef() << "\n";
 }
 
+void DynTypedNode::dump() const {
+  if (const Decl *D = get())
+D->dump();
+  else if (const Stmt *S = get())
+S->dump();
+  else if (const Type *T = get())
+T->dump();
+  else
+llvm::errs() << "Unable to dump values of type " << NodeKind.asStringRef()
+ << "\n";
+}
+
 SourceRange DynTypedNode::getSourceRange() const {
   if (const CXXCtorInitializer *CCI = get())
 return CCI->getSourceRange();
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -296,6 +296,9 @@
   /// Dumps the node to the given output stream.
   void dump(llvm::raw_ostream , const ASTContext ) const;
 
+  /// Dumps the node to \c llvm::errs().
+  void dump() const;
+
   /// For nodes which represent textual entities in the source code,
   /// return their SourceRange.  For all other nodes, return SourceRange().
   SourceRange getSourceRange() const;
Index: clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
@@ -430,6 +430,9 @@
 // be fine since for the statements we care about there should only be one
 // parent, except for the case specified below.
 
+llvm::errs() << "DUMP\n";
+Start.dump();
+
 assert(MacroLoc.isFileID());
 
 while (true) {


Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ clang/lib/AST/ASTTypeTraits.cpp
@@ -213,6 +213,18 @@
 OS << "Unable to dump values of type " << NodeKind.asStringRef() << "\n";
 }
 
+void DynTypedNode::dump() const {
+  if (const Decl *D = get())
+D->dump();
+  else if (const Stmt *S = get())
+S->dump();
+  else if (const Type *T = get())
+T->dump();
+  else
+llvm::errs() << "Unable to dump values of type " << NodeKind.asStringRef()
+ << "\n";
+}
+
 SourceRange DynTypedNode::getSourceRange() const {
   if (const CXXCtorInitializer *CCI = get())
 return CCI->getSourceRange();
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -296,6 +296,9 @@
   /// Dumps the node to the given output stream.
   void dump(llvm::raw_ostream , const ASTContext ) const;
 
+  /// Dumps the node to \c llvm::errs().
+  void dump() const;
+
   /// For nodes which represent textual entities in the source code,
   /// return their SourceRange.  For all other nodes, return SourceRange().
   SourceRange getSourceRange() const;
Index: clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
@@ -430,6 +430,9 @@
 // be fine since for the statements we care about there should only be one
 // parent, except for the case specified below.
 
+llvm::errs() << "DUMP\n";
+Start.dump();
+
 assert(MacroLoc.isFileID());
 
 while (true) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141570: [clang] Add no-argument dump to DynTypedNode

[Chris Cotter via Phabricator via cfe-commits]

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

Provide a no-argument dump methods to avoid having to do
additional gymnastics to dump a DynTypedNode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141570

Files:
  clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
  clang/include/clang/AST/ASTTypeTraits.h
  clang/lib/AST/ASTTypeTraits.cpp


Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ clang/lib/AST/ASTTypeTraits.cpp
@@ -213,6 +213,17 @@
 OS << "Unable to dump values of type " << NodeKind.asStringRef() << "\n";
 }
 
+void DynTypedNode::dump() const {
+  if (const Decl *D = get())
+D->dump();
+  else if (const Stmt *S = get())
+S->dump();
+  else if (const Type *T = get())
+T->dump();
+  else
+llvm::errs() << "Unable to dump values of type " << NodeKind.asStringRef() 
<< "\n";
+}
+
 SourceRange DynTypedNode::getSourceRange() const {
   if (const CXXCtorInitializer *CCI = get())
 return CCI->getSourceRange();
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -296,6 +296,9 @@
   /// Dumps the node to the given output stream.
   void dump(llvm::raw_ostream , const ASTContext ) const;
 
+  /// Dumps the node to \c llvm::errs().
+  void dump() const;
+
   /// For nodes which represent textual entities in the source code,
   /// return their SourceRange.  For all other nodes, return SourceRange().
   SourceRange getSourceRange() const;
Index: clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
@@ -430,6 +430,9 @@
 // be fine since for the statements we care about there should only be one
 // parent, except for the case specified below.
 
+llvm::errs() << "DUMP\n";
+Start.dump();
+
 assert(MacroLoc.isFileID());
 
 while (true) {


Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ clang/lib/AST/ASTTypeTraits.cpp
@@ -213,6 +213,17 @@
 OS << "Unable to dump values of type " << NodeKind.asStringRef() << "\n";
 }
 
+void DynTypedNode::dump() const {
+  if (const Decl *D = get())
+D->dump();
+  else if (const Stmt *S = get())
+S->dump();
+  else if (const Type *T = get())
+T->dump();
+  else
+llvm::errs() << "Unable to dump values of type " << NodeKind.asStringRef() << "\n";
+}
+
 SourceRange DynTypedNode::getSourceRange() const {
   if (const CXXCtorInitializer *CCI = get())
 return CCI->getSourceRange();
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -296,6 +296,9 @@
   /// Dumps the node to the given output stream.
   void dump(llvm::raw_ostream , const ASTContext ) const;
 
+  /// Dumps the node to \c llvm::errs().
+  void dump() const;
+
   /// For nodes which represent textual entities in the source code,
   /// return their SourceRange.  For all other nodes, return SourceRange().
   SourceRange getSourceRange() const;
Index: clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
@@ -430,6 +430,9 @@
 // be fine since for the statements we care about there should only be one
 // parent, except for the case specified below.
 
+llvm::errs() << "DUMP\n";
+Start.dump();
+
 assert(MacroLoc.isFileID());
 
 while (true) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d851a79 - [NFC] Refactor the outdated warning message about removing std::experimental::coroutine

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-01-12T15:32:40+08:00
New Revision: d851a7937e7669cf1f04768ffb4593ba179fd32a

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

LOG: [NFC] Refactor the outdated warning message about removing 
std::experimental::coroutine

The warning message is out of date. According to
https://github.com/llvm/llvm-project/issues/59110 and
https://reviews.llvm.org/D108697, this would be removed in LLVM17.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7154a8bcbde47..3426921994858 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11274,7 +11274,7 @@ def err_implied_coroutine_type_not_found : Error<
   "a coroutine; include  if your version "
   "of libcxx is less than 14.0">;
 def warn_deprecated_coroutine_namespace : Warning<
-  "support for std::experimental::%0 will be removed in LLVM 15; "
+  "support for std::experimental::%0 will be removed in LLVM 17; "
   "use std::%0 instead">,
   InGroup;
 def err_mixed_use_std_and_experimental_namespace_for_coroutine : Error<



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

[PATCH] D129570: [clang-tidy] Add new clang-tidy check to find implicit conversions from enum to integer.

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.cpp:45
+  } else {
+Note << FixItHint::CreateInsertion(MatchedExpr->getBeginLoc(), "(int)");
+  }

pfultz2 wrote:
> carlosgalvezp wrote:
> > I don't think we can assume the type of the enum is `int`, see for example:
> > 
> > ```
> > enum Foo : unsigned int
> > {
> > a,
> > b
> > };
> > 
> > 
> > void f(unsigned int);
> > 
> > int main()
> > {
> > f(a);
> > }
> > 
> > ```
> > 
> > Even if there is no explicit underlying type, isn't the underlying type 
> > implementation-defined?
> Since its an explicit cast then we should probably use the type that the 
> function accepts(since thats what it will be eventually converted to)  rather 
> than the type of the underling enum type.
I'm not sure that's correct either, and it could lead to narrowing conversion 
warnings being ignored, see example:

https://godbolt.org/z/PfPxEYPrj

The correct thing to do would be to cast to the underlying type of the enum. A 
user could do it like this, although it's quite unreadable:

  static_cast>(enum_value)

But since in this context we have a lot more information from the AST, maybe we 
can automatically figure out the underlying type of the enum without having to 
call `std::underlying_type`?




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

https://reviews.llvm.org/D129570

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

[PATCH] D141569: Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter created this revision.
Herald added subscribers: carlosgalvezp, shchenz, kbarton, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
ccotter requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Warn when an rvalue reference function paramter is never moved
from within the function body.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141569

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
@@ -0,0 +1,224 @@
+// RUN: %check_clang_tidy -std=c++14-or-later %s cppcoreguidelines-rvalue-reference-param-not-moved %t
+
+// NOLINTBEGIN
+namespace std {
+template 
+struct remove_reference;
+
+template 
+struct remove_reference {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &> {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &&> {
+  typedef _Tp type;
+};
+
+template 
+constexpr typename std::remove_reference<_Tp>::type &(_Tp &&__t) noexcept {
+  return static_cast::type &&>(__t);
+}
+
+template 
+constexpr _Tp &&
+forward(typename remove_reference<_Tp>::type &__t) noexcept {
+  return static_cast<_Tp &&>(__t);
+}
+
+}
+// NOLINTEND
+
+struct Obj {
+  Obj();
+  Obj(const Obj&);
+  Obj& operator=(const Obj&);
+  Obj(Obj&&);
+  Obj& operator=(Obj&&);
+  void member() const;
+};
+
+void consumes_object(Obj);
+
+void never_moves_param(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  o.member();
+}
+
+void copies_object(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  Obj copy = o;
+}
+
+template 
+void never_moves_param_template(Obj&& o, T t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  o.member();
+}
+
+void never_moves_params(Obj&& o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  // CHECK-MESSAGES: :[[@LINE-2]]:35: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void never_moves_some_params(Obj&& o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+  Obj other{std::move(o2)};
+}
+
+void never_moves_mixed(Obj o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void lambda_captures_parameter_as_value(Obj&& o) {
+  auto f = [o]() {
+consumes_object(std::move(o));
+  };
+  // CHECK-MESSAGES: :[[@LINE-4]]:41: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void lambda_captures_parameter_as_value_nested(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  auto f = []() {
+auto f_nested = [o]() {
+  consumes_object(std::move(o));
+};
+  };
+  auto f2 = [o]() {
+auto f_nested = []() {
+  consumes_object(std::move(o));
+};
+  };
+  auto f3 = [o]() {
+auto f_nested = []() {
+  auto f_nested_inner = []() {
+consumes_object(std::move(o));
+  };
+};
+  };
+  auto f4 = []() {
+auto f_nested = []() {
+  auto f_nested_inner = [o]() {
+consumes_object(std::move(o));
+  };
+};
+  };
+}
+
+void misc_lambda_checks() {
+  auto never_moves = [](Obj&& o1) {
+Obj other{o1};
+  };
+  // CHECK-MESSAGES: :[[@LINE-3]]:25: warning: rvalue 

[PATCH] D141546: [clang] Reland parenthesized aggregate init patches

[Alan Zhao via Phabricator via cfe-commits]

ayzhao updated this revision to Diff 488486.
ayzhao added a comment.

fix spelling + improve diagnostics


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141546

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/ComputeDependence.h
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Initialization.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/p4.cpp
  clang/test/CodeGen/paren-list-agg-init.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/PCH/cxx_paren_init.cpp
  clang/test/PCH/cxx_paren_init.h
  clang/test/SemaCXX/cxx2a-explicit-bool.cpp
  clang/test/SemaCXX/paren-list-agg-init.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1156,7 +1156,7 @@
 
   Parenthesized initialization of aggregates
   https://wg21.link/p0960r3;>P0960R3
-  No
+  Clang 16
 

 https://wg21.link/p1975r0;>P1975R0
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -643,6 +643,10 @@
 K = CXCursor_RequiresExpr;
 break;
 
+  case Stmt::CXXParenListInitExprClass:
+K = CXCursor_CXXParenListInitExpr;
+break;
+
   case Stmt::MSDependentExistsStmtClass:
 K = CXCursor_UnexposedStmt;
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2139,6 +2139,7 @@
   void VisitLambdaExpr(const LambdaExpr *E);
   void VisitConceptSpecializationExpr(const ConceptSpecializationExpr *E);
   void VisitRequiresExpr(const RequiresExpr *E);
+  void VisitCXXParenListInitExpr(const CXXParenListInitExpr *E);
   void VisitOMPExecutableDirective(const OMPExecutableDirective *D);
   void VisitOMPLoopBasedDirective(const OMPLoopBasedDirective *D);
   void VisitOMPLoopDirective(const OMPLoopDirective *D);
@@ -3006,6 +3007,9 @@
   for (ParmVarDecl *VD : E->getLocalParameters())
 AddDecl(VD);
 }
+void EnqueueVisitor::VisitCXXParenListInitExpr(const CXXParenListInitExpr *E) {
+  EnqueueChildren(E);
+}
 void EnqueueVisitor::VisitPseudoObjectExpr(const PseudoObjectExpr *E) {
   // Treat the expression like its syntactic form.
   Visit(E->getSyntacticForm());
@@ -5587,6 +5591,8 @@
 return cxstring::createRef("ConceptSpecializationExpr");
   case CXCursor_RequiresExpr:
 return cxstring::createRef("RequiresExpr");
+  case CXCursor_CXXParenListInitExpr:
+return cxstring::createRef("CXXParenListInitExpr");
   case CXCursor_UnexposedStmt:
 return cxstring::createRef("UnexposedStmt");
   case CXCursor_DeclStmt:
Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -frecovery-ast -frecovery-ast-type -o - %s -std=gnu++17 -fsyntax-only -verify
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -o - %s -std=gnu++17 -fsyntax-only -verify
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -o - %s -std=gnu++20 -fsyntax-only -verify
+
 
 namespace test0 {
 struct Indestructible {
@@ -171,3 +173,13 @@
   S.m(1); // no crash
 }
 }
+
+namespace test16 {
+// verify we do not crash on incomplete class type.
+template struct A; // expected-note 5{{template is declared here}}
+A foo() { // expected-error {{implicit instantiation of undefined template}}
+  if (1 == 1)
+return A{1}; // expected-error 2{{implicit instantiation of 

[PATCH] D141531: [OpenMP][FIX] Allow multiple `depend` clauses on a `taskwait nowait`

[Johannes Doerfert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa8eccca93415: [OpenMP][FIX] Allow multiple `depend` clauses 
on a `taskwait nowait` (authored by jdoerfert).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141531

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/taskwait_ast_print.cpp


Index: clang/test/OpenMP/taskwait_ast_print.cpp
===
--- clang/test/OpenMP/taskwait_ast_print.cpp
+++ clang/test/OpenMP/taskwait_ast_print.cpp
@@ -25,6 +25,7 @@
   static T a;
 #pragma omp taskwait
 #pragma omp taskwait depend(in:a, argc) nowait
+#pragma omp taskwait depend(in:a) depend(in:argc) nowait
   return a + argc;
 }
 
@@ -41,12 +42,15 @@
 // CHECK:  static T a;
 // CHECK-NEXT: #pragma omp taskwait{{$}}
 // CHECK-NEXT: #pragma omp taskwait depend(in : a,argc) nowait{{$}}
+// CHECK-NEXT: #pragma omp taskwait depend(in : a) depend(in : argc) 
nowait{{$}}
 // CHECK:  static int a;
 // CHECK-NEXT: #pragma omp taskwait
 // CHECK-NEXT: #pragma omp taskwait depend(in : a,argc) nowait{{$}}
+// CHECK-NEXT: #pragma omp taskwait depend(in : a) depend(in : argc) 
nowait{{$}}
 // CHECK:  static char a;
 // CHECK-NEXT: #pragma omp taskwait
 // CHECK-NEXT: #pragma omp taskwait depend(in : a,argc) nowait{{$}}
+// CHECK-NEXT: #pragma omp taskwait depend(in : a) depend(in : argc) 
nowait{{$}}
 
 int main(int argc, char **argv) {
   static int a;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -11109,9 +11109,10 @@
   SourceLocation EndLoc) {
   const OMPNowaitClause *NowaitC =
   OMPExecutableDirective::getSingleClause(Clauses);
-  const OMPDependClause *DependC =
-  OMPExecutableDirective::getSingleClause(Clauses);
-  if (NowaitC && !DependC) {
+  bool HasDependC =
+  !OMPExecutableDirective::getClausesOfKind(Clauses)
+   .empty();
+  if (NowaitC && !HasDependC) {
 Diag(StartLoc, diag::err_omp_nowait_clause_without_depend);
 return StmtError();
   }


Index: clang/test/OpenMP/taskwait_ast_print.cpp
===
--- clang/test/OpenMP/taskwait_ast_print.cpp
+++ clang/test/OpenMP/taskwait_ast_print.cpp
@@ -25,6 +25,7 @@
   static T a;
 #pragma omp taskwait
 #pragma omp taskwait depend(in:a, argc) nowait
+#pragma omp taskwait depend(in:a) depend(in:argc) nowait
   return a + argc;
 }
 
@@ -41,12 +42,15 @@
 // CHECK:  static T a;
 // CHECK-NEXT: #pragma omp taskwait{{$}}
 // CHECK-NEXT: #pragma omp taskwait depend(in : a,argc) nowait{{$}}
+// CHECK-NEXT: #pragma omp taskwait depend(in : a) depend(in : argc) nowait{{$}}
 // CHECK:  static int a;
 // CHECK-NEXT: #pragma omp taskwait
 // CHECK-NEXT: #pragma omp taskwait depend(in : a,argc) nowait{{$}}
+// CHECK-NEXT: #pragma omp taskwait depend(in : a) depend(in : argc) nowait{{$}}
 // CHECK:  static char a;
 // CHECK-NEXT: #pragma omp taskwait
 // CHECK-NEXT: #pragma omp taskwait depend(in : a,argc) nowait{{$}}
+// CHECK-NEXT: #pragma omp taskwait depend(in : a) depend(in : argc) nowait{{$}}
 
 int main(int argc, char **argv) {
   static int a;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -11109,9 +11109,10 @@
   SourceLocation EndLoc) {
   const OMPNowaitClause *NowaitC =
   OMPExecutableDirective::getSingleClause(Clauses);
-  const OMPDependClause *DependC =
-  OMPExecutableDirective::getSingleClause(Clauses);
-  if (NowaitC && !DependC) {
+  bool HasDependC =
+  !OMPExecutableDirective::getClausesOfKind(Clauses)
+   .empty();
+  if (NowaitC && !HasDependC) {
 Diag(StartLoc, diag::err_omp_nowait_clause_without_depend);
 return StmtError();
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141540: [OpenMP][5.1] Support `thread_limit` on `omp target`

[Johannes Doerfert 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 rG5e4369e53d3c: [OpenMP][5.1] Support `thread_limit` on `omp 
target` (authored by jdoerfert).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D141540?vs=488337=488480#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141540

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_ast_print.cpp
  clang/test/OpenMP/target_codegen.cpp

Index: clang/test/OpenMP/target_codegen.cpp
===
--- clang/test/OpenMP/target_codegen.cpp
+++ clang/test/OpenMP/target_codegen.cpp
@@ -13,6 +13,13 @@
 // RUN: %clang_cc1 -no-opaque-pointers -fopenmp -x c++ -std=c++11 -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-pch -o %t %s
 // RUN: %clang_cc1 -no-opaque-pointers -fopenmp -x c++ -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32 --check-prefix OMP50
 
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -fopenmp-version=51 -D_DOMP51 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64 --check-prefix OMP51
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp -x c++ -fopenmp-version=51 -D_DOMP51 -std=c++11 -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-pch -o %t %s
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp -x c++ -fopenmp-version=51 -D_DOMP51 -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64 --check-prefix OMP51
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -fopenmp-version=51 -D_DOMP51 -x c++ -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32 --check-prefix OMP51
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp -fopenmp-version=51 -D_DOMP51 -x c++ -std=c++11 -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-pch -o %t %s
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp -fopenmp-version=51 -D_DOMP51 -x c++ -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32 --check-prefix OMP51
+
 // RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp-simd -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -no-opaque-pointers -fopenmp-simd -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-pch -o %t %s
 // RUN: %clang_cc1 -no-opaque-pointers -fopenmp-simd -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
@@ -902,6 +909,52 @@
   }
 };
 
+#ifdef _DOMP51
+void thread_limit_target(int TargetTL, int TeamsTL) {
+
+#pragma omp target
+{}
+// OMP51: call i32 @__tgt_target_kernel({{.*}}, i64 -1, i32 -1, i32 0,
+
+#pragma omp target
+#pragma omp teams
+{}
+// OMP51: call i32 @__tgt_target_kernel({{.*}}, i64 -1, i32 0, i32 0,
+
+#pragma omp target thread_limit(TargetTL)
+{}
+// OMP51: [[TL:%.*]] = load {{.*}} %TargetTL.addr
+// OMP51: store {{.*}} [[TL]], {{.*}} [[CEA:%.*]]
+// OMP51: load {{.*}} [[CEA]]
+// OMP51: [[CE:%.*]] = load {{.*}} [[CEA]]
+// OMP51: call i32 @__tgt_target_kernel({{.*}}, i64 -1, i32 -1, i32 [[CE]],
+
+#pragma omp target thread_limit(TargetTL)
+#pragma omp teams
+{}
+// OMP51: [[TL:%.*]] = load {{.*}} %TargetTL.addr
+// OMP51: store {{.*}} [[TL]], {{.*}} [[CEA:%.*]]
+// OMP51: load {{.*}} [[CEA]]
+// OMP51: [[CE:%.*]] = load {{.*}} [[CEA]]
+// OMP51: call i32 @__tgt_target_kernel({{.*}}, i64 -1, i32 0, i32 [[CE]],
+
+#pragma omp target
+#pragma omp teams thread_limit(TeamsTL)
+{}
+// OMP51: load {{.*}} %TeamsTL.addr
+// OMP51: [[TeamsL:%.*]] = load {{.*}} %TeamsTL.addr
+// OMP51: call i32 @__tgt_target_kernel({{.*}}, i64 -1, i32 0, i32 [[TeamsL]],
+
+#pragma omp target thread_limit(TargetTL)
+#pragma omp teams thread_limit(TeamsTL)
+{}
+// OMP51: load {{.*}} %TeamsTL.addr
+// OMP51: [[TeamsL:%.*]] = load {{.*}} %TeamsTL.addr
+// OMP51: call i32 @__tgt_target_kernel({{.*}}, i64 -1, i32 0, i32 [[TeamsL]],
+
+}
+#endif
+
 // CHECK: define internal void @.omp_offloading.requires_reg()
 // CHECK: call void @__tgt_register_requires(i64 

[clang] 5e4369e - [OpenMP][5.1] Support `thread_limit` on `omp target`

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2023-01-11T22:24:23-08:00
New Revision: 5e4369e53d3c33b3ec69c0b7ec180db8851e792a

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

LOG: [OpenMP][5.1] Support `thread_limit` on `omp target`

It is unclear to me what happens if we have two thread_limit clauses to
choose from. I will recommend to the standards committee to disallow
that. For now, we pick the teams one.

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

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/target_ast_print.cpp
clang/test/OpenMP/target_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index c2328d28ec50a..2b99b302c38d0 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6429,10 +6429,8 @@ static llvm::Value *getNumThreads(CodeGenFunction , 
const CapturedStmt *CS,
 }
 if (isOpenMPSimdDirective(Dir->getDirectiveKind()))
   return CGF.Builder.getInt32(1);
-return DefaultThreadLimitVal;
   }
-  return DefaultThreadLimitVal ? DefaultThreadLimitVal
-   : CGF.Builder.getInt32(0);
+  return DefaultThreadLimitVal;
 }
 
 const Expr *CGOpenMPRuntime::getNumThreadsExprForTargetDirective(
@@ -6575,12 +6573,14 @@ llvm::Value 
*CGOpenMPRuntime::emitNumThreadsForTargetDirective(
   return NumThreads;
 const Stmt *Child = CGOpenMPRuntime::getSingleCompoundChild(
 CGF.getContext(), CS->getCapturedStmt());
+// TODO: The standard is not clear how to resolve two thread limit clauses,
+//   let's pick the teams one if it's present, otherwise the target 
one.
+const auto *ThreadLimitClause = D.getSingleClause();
 if (const auto *Dir = dyn_cast_or_null(Child)) {
-  if (Dir->hasClausesOfKind()) {
+  if (const auto *TLC = Dir->getSingleClause()) {
+ThreadLimitClause = TLC;
 CGOpenMPInnerExprInfo CGInfo(CGF, *CS);
 CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(CGF, );
-const auto *ThreadLimitClause =
-Dir->getSingleClause();
 CodeGenFunction::LexicalScope Scope(
 CGF, ThreadLimitClause->getThreadLimit()->getSourceRange());
 if (const auto *PreInit =
@@ -6595,11 +6595,15 @@ llvm::Value 
*CGOpenMPRuntime::emitNumThreadsForTargetDirective(
 }
   }
 }
-llvm::Value *ThreadLimit = CGF.EmitScalarExpr(
-ThreadLimitClause->getThreadLimit(), /*IgnoreResultAssign=*/true);
-ThreadLimitVal =
-Bld.CreateIntCast(ThreadLimit, CGF.Int32Ty, /*isSigned=*/false);
   }
+}
+if (ThreadLimitClause) {
+  llvm::Value *ThreadLimit = CGF.EmitScalarExpr(
+  ThreadLimitClause->getThreadLimit(), /*IgnoreResultAssign=*/true);
+  ThreadLimitVal =
+  Bld.CreateIntCast(ThreadLimit, CGF.Int32Ty, /*isSigned=*/false);
+}
+if (const auto *Dir = dyn_cast_or_null(Child)) {
   if (isOpenMPTeamsDirective(Dir->getDirectiveKind()) &&
   !isOpenMPDistributeDirective(Dir->getDirectiveKind())) {
 CS = Dir->getInnermostCapturedStmt();
@@ -6650,7 +6654,10 @@ llvm::Value 
*CGOpenMPRuntime::emitNumThreadsForTargetDirective(
   ThreadLimitVal =
   Bld.CreateIntCast(ThreadLimit, CGF.Int32Ty, /*isSigned=*/false);
 }
-return getNumThreads(CGF, D.getInnermostCapturedStmt(), ThreadLimitVal);
+if (llvm::Value *NumThreads =
+getNumThreads(CGF, D.getInnermostCapturedStmt(), ThreadLimitVal))
+  return NumThreads;
+return Bld.getInt32(0);
   case OMPD_target_parallel:
   case OMPD_target_parallel_for:
   case OMPD_target_parallel_for_simd:

diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index ed7d8998a6ede..e6035578a9cde 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -15635,6 +15635,7 @@ static OpenMPDirectiveKind 
getOpenMPCaptureRegionForClause(
 break;
   case OMPC_thread_limit:
 switch (DKind) {
+case OMPD_target:
 case OMPD_target_teams:
 case OMPD_target_teams_distribute:
 case OMPD_target_teams_distribute_simd:
@@ -15676,7 +15677,6 @@ static OpenMPDirectiveKind 
getOpenMPCaptureRegionForClause(
 case OMPD_parallel_for:
 case OMPD_parallel_for_simd:
 case OMPD_parallel_loop:
-case OMPD_target:
 case OMPD_target_simd:
 case OMPD_target_parallel:
 case OMPD_target_parallel_for:

diff  --git a/clang/test/OpenMP/target_ast_print.cpp 
b/clang/test/OpenMP/target_ast_print.cpp
index aae5fb681ace7..c1f37fa5b1726 100644
--- a/clang/test/OpenMP/target_ast_print.cpp
+++ 

[clang] a8eccca - [OpenMP][FIX] Allow multiple `depend` clauses on a `taskwait nowait`

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2023-01-11T22:24:23-08:00
New Revision: a8eccca934156943f02c4423e4b7df12fe77dca8

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

LOG: [OpenMP][FIX] Allow multiple `depend` clauses on a `taskwait nowait`

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

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

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/taskwait_ast_print.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index e6035578a9cde..f48707d73b734 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -11109,9 +11109,10 @@ StmtResult 
Sema::ActOnOpenMPTaskwaitDirective(ArrayRef Clauses,
   SourceLocation EndLoc) {
   const OMPNowaitClause *NowaitC =
   OMPExecutableDirective::getSingleClause(Clauses);
-  const OMPDependClause *DependC =
-  OMPExecutableDirective::getSingleClause(Clauses);
-  if (NowaitC && !DependC) {
+  bool HasDependC =
+  !OMPExecutableDirective::getClausesOfKind(Clauses)
+   .empty();
+  if (NowaitC && !HasDependC) {
 Diag(StartLoc, diag::err_omp_nowait_clause_without_depend);
 return StmtError();
   }

diff  --git a/clang/test/OpenMP/taskwait_ast_print.cpp 
b/clang/test/OpenMP/taskwait_ast_print.cpp
index 670e1fbe7fc76..10823c6cd782c 100644
--- a/clang/test/OpenMP/taskwait_ast_print.cpp
+++ b/clang/test/OpenMP/taskwait_ast_print.cpp
@@ -25,6 +25,7 @@ T ndmain(T argc) {
   static T a;
 #pragma omp taskwait
 #pragma omp taskwait depend(in:a, argc) nowait
+#pragma omp taskwait depend(in:a) depend(in:argc) nowait
   return a + argc;
 }
 
@@ -41,12 +42,15 @@ T ndmain(T argc) {
 // CHECK:  static T a;
 // CHECK-NEXT: #pragma omp taskwait{{$}}
 // CHECK-NEXT: #pragma omp taskwait depend(in : a,argc) nowait{{$}}
+// CHECK-NEXT: #pragma omp taskwait depend(in : a) depend(in : argc) 
nowait{{$}}
 // CHECK:  static int a;
 // CHECK-NEXT: #pragma omp taskwait
 // CHECK-NEXT: #pragma omp taskwait depend(in : a,argc) nowait{{$}}
+// CHECK-NEXT: #pragma omp taskwait depend(in : a) depend(in : argc) 
nowait{{$}}
 // CHECK:  static char a;
 // CHECK-NEXT: #pragma omp taskwait
 // CHECK-NEXT: #pragma omp taskwait depend(in : a,argc) nowait{{$}}
+// CHECK-NEXT: #pragma omp taskwait depend(in : a) depend(in : argc) 
nowait{{$}}
 
 int main(int argc, char **argv) {
   static int a;



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

[PATCH] D141105: [OpenMP] Add support for '--offload-arch=native' to OpenMP offloading

[Caroline Tice via Phabricator via cfe-commits]

cmtice added a comment.

Just a heads up: This commit is causing our bootstrap build to fail (your new 
openmp-system-arch.c test is failing in our stage1 compiler).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141105

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

[PATCH] D137487: [clang][Interp] Start implementing builtin functions

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/Interp/Interp.h:1315
+  if (InterpretBuiltin(S, PC, Func->getBuiltinID())) {
+NewFrame.release();
+return true;

We don't have to update `S.Current`?



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:20
+template ::T>
+static bool Ret(InterpState , CodePtr ) {
+  S.CallStackDepth--;

Why not just factor out `Ret` now?


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

https://reviews.llvm.org/D137487

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

[PATCH] D138802: [clang][Interp] Implement DecompositionDecls

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:291
   /// Returns whether we should create a global variable for the
   /// given VarDecl.
+  bool isGlobalDecl(const ValueDecl *VD) const {





Comment at: clang/test/AST/Interp/cxx17.cpp:7
+
+struct F { int a; int b;};
+constexpr F getF() {

It would also be good to test references, bit-fields, volatile and tuple-like 
types if possible, if not then leave a FIXME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138802

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

[PATCH] D141215: [clang-repl][WIP] Implement pretty printing

[Jun Zhang via Phabricator via cfe-commits]

junaire updated this revision to Diff 488466.
junaire added a comment.

Let's try a better approach to determine whether we should pretty print the 
expression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Parse/Parser.h
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/PrettyPrint.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/tools/clang-repl/CMakeLists.txt
  clang/tools/clang-repl/ClangRepl.cpp

Index: clang/tools/clang-repl/ClangRepl.cpp
===
--- clang/tools/clang-repl/ClangRepl.cpp
+++ clang/tools/clang-repl/ClangRepl.cpp
@@ -10,6 +10,7 @@
 //
 //===--===//
 
+#include "clang/AST/Decl.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
@@ -18,6 +19,8 @@
 #include "llvm/ExecutionEngine/Orc/LLJIT.h"
 #include "llvm/LineEditor/LineEditor.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h" // llvm_shutdown
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h" // llvm::Initialize*
@@ -65,6 +68,18 @@
   return (Errs || HasError) ? EXIT_FAILURE : EXIT_SUCCESS;
 }
 
+static void DeclareMagicFunctions(clang::Interpreter ) {
+  llvm::ArrayRef MagicFunctions = {
+  "void __InterpreterPrettyPrint(void*, char);",
+  "void __InterpreterPrettyPrint(void*, int);",
+  "void __InterpreterPrettyPrint(void*, float);",
+  "void __InterpreterPrettyPrint(void*, double);",
+  };
+  for (llvm::StringRef Function : MagicFunctions) {
+llvm::cantFail(Interp.ParseAndExecute(Function));
+  }
+}
+
 llvm::ExitOnError ExitOnErr;
 int main(int argc, const char **argv) {
   ExitOnErr.setBanner("clang-repl: ");
@@ -109,13 +124,18 @@
 
   bool HasError = false;
 
+  DeclareMagicFunctions(*Interp);
   if (OptInputs.empty()) {
 llvm::LineEditor LE("clang-repl");
 // FIXME: Add LE.setListCompleter
 while (llvm::Optional Line = LE.readLine()) {
-  if (*Line == R"(%quit)")
+  llvm::StringRef Code = *Line;
+  if (Code.empty()) {
+continue;
+  }
+  if (Code == R"(%quit)")
 break;
-  if (*Line == R"(%undo)") {
+  if (Code == R"(%undo)") {
 if (auto Err = Interp->Undo()) {
   llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
   HasError = true;
@@ -123,7 +143,7 @@
 continue;
   }
 
-  if (auto Err = Interp->ParseAndExecute(*Line)) {
+  if (auto Err = Interp->ParseAndExecute(Code)) {
 llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
 HasError = true;
   }
Index: clang/tools/clang-repl/CMakeLists.txt
===
--- clang/tools/clang-repl/CMakeLists.txt
+++ clang/tools/clang-repl/CMakeLists.txt
@@ -12,6 +12,7 @@
   )
 
 clang_target_link_libraries(clang-repl PRIVATE
+  clangAST
   clangBasic
   clangFrontend
   clangInterpreter
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -153,10 +153,19 @@
   return true;
 }
 
-bool Parser::ExpectAndConsumeSemi(unsigned DiagID, StringRef TokenUsed) {
+bool Parser::ExpectAndConsumeSemi(unsigned DiagID, StringRef TokenUsed, bool IsTopExpr) {
   if (TryConsumeToken(tok::semi))
 return false;
 
+  // If this is in the incremental C++ mode, then it means we need to pretty
+  // print this expression. Thus, let's pretend we have this semi and continue
+  // parsing.
+  if (PP.isIncrementalProcessingEnabled() &&
+  IsTopExpr && DiagID == diag::err_expected_semi_after_expr) {
+setPrettyPrintMode();
+return false;
+  }
+
   if (Tok.is(tok::code_completion)) {
 handleUnexpectedCodeCompletionToken();
 return false;
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -541,9 +541,8 @@
 // Recover parsing as a case statement.
 return ParseCaseStatement(StmtCtx, /*MissingCase=*/true, Expr);
   }
-
   // Otherwise, eat the semicolon.
-  ExpectAndConsumeSemi(diag::err_expected_semi_after_expr);
+  ExpectAndConsumeSemi(diag::err_expected_semi_after_expr, /*TokenUsed=*/"", StmtCtx == ParsedStmtContext::SubStmt);
   return handleExprStmt(Expr, StmtCtx);
 }
 
Index: clang/lib/Interpreter/PrettyPrint.cpp

RE: [PATCH] D141437: [HIP] Use .hipi as preprocessor output extension

[Liu, Yaxun (Sam) via cfe-commits]

[AMD Official Use Only - General]

Fixed by 503e02e861662ef84940fdc05e26c12fc0fca7f3. Thanks.

Sam

-Original Message-
From: Nico Weber via Phabricator 
Sent: Wednesday, January 11, 2023 11:25 PM
To: Liu, Yaxun (Sam) ; t...@google.com
Cc: tha...@chromium.org; mask...@google.com; cfe-commits@lists.llvm.org; 
473750...@qq.com; tul...@redhat.com; yronglin...@gmail.com; Kumar N, 
Bhuvanendra ; 1135831...@qq.com; 
gandhi21...@gmail.com; tbae...@redhat.com; mlek...@skidmore.edu; 
blitzrak...@gmail.com; shen...@google.com
Subject: [PATCH] D141437: [HIP] Use .hipi as preprocessor output extension

Caution: This message originated from an External Source. Use proper caution 
when opening attachments, clicking links, or responding.


thakis added a comment.

I think this breaks tests on Windows: 
https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2F45.33.8.238%2Fwin%2F73191%2Fstep_7.txt=05%7C01%7Cyaxun.liu%40amd.com%7C13de06bdc38e49edd6f308daf4550754%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638090943311573611%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=JihDs%2FYNuK5FqlQnlpeH6HHfMuVykklmOg0MgUbJ4A8%3D=0

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD141437%2Fnew%2F=05%7C01%7Cyaxun.liu%40amd.com%7C13de06bdc38e49edd6f308daf4550754%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638090943311573611%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=36LDFan8FQ%2BnroFc8LAaN5dJENQeMvb6eytkNaRHzps%3D=0

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD141437=05%7C01%7Cyaxun.liu%40amd.com%7C13de06bdc38e49edd6f308daf4550754%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638090943311573611%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=J4SCWar%2BKIF2eMoXtxHz1LSrgoxCU7NoPCBmQFxVg7U%3D=0

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

[clang] 503e02e - Fix test hip-windows-filename.hip

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2023-01-11T23:31:49-05:00
New Revision: 503e02e861662ef84940fdc05e26c12fc0fca7f3

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

LOG: Fix test hip-windows-filename.hip

due to https://reviews.llvm.org/D141437

Added: 


Modified: 
clang/test/Driver/hip-windows-filename.hip

Removed: 




diff  --git a/clang/test/Driver/hip-windows-filename.hip 
b/clang/test/Driver/hip-windows-filename.hip
index d5318e2a111c5..39534a006b4a8 100644
--- a/clang/test/Driver/hip-windows-filename.hip
+++ b/clang/test/Driver/hip-windows-filename.hip
@@ -6,5 +6,5 @@
 // RUN:   -nogpuinc -nogpulib -save-temps \
 // RUN:   %s 2>&1 | FileCheck %s
 
-// CHECK: "-o" "hip-windows-filename-hip-amdgcn-amd-amdhsa-gfx908@xnack+.cui"
-// CHECK-NOT: "-o" 
"hip-windows-filename-hip-amdgcn-amd-amdhsa-gfx908:xnack+.cui"
+// CHECK: "-o" "hip-windows-filename-hip-amdgcn-amd-amdhsa-gfx908@xnack+.hipi"
+// CHECK-NOT: "-o" 
"hip-windows-filename-hip-amdgcn-amd-amdhsa-gfx908:xnack+.hipi"



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

[PATCH] D141437: [HIP] Use .hipi as preprocessor output extension

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

I think this breaks tests on Windows: http://45.33.8.238/win/73191/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141437

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

[PATCH] D141563: [clang-format] Fix a bug in DerivePointerAlignment fallback

[Owen Pan via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141563

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10781,6 +10781,33 @@
 format(Prefix + "int* x;", DerivePointerAlignment));
 }
 
+TEST_F(FormatTest, PointerAlignmentFallback) {
+  FormatStyle Style = getLLVMStyle();
+  Style.DerivePointerAlignment = true;
+
+  const StringRef Code("int* p;\n"
+   "int *q;\n"
+   "int * r;");
+
+  EXPECT_EQ(Style.PointerAlignment, FormatStyle::PAS_Right);
+  verifyFormat("int *p;\n"
+   "int *q;\n"
+   "int *r;",
+   Code, Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("int* p;\n"
+   "int* q;\n"
+   "int* r;",
+   Code, Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("int * p;\n"
+   "int * q;\n"
+   "int * r;",
+   Code, Style);
+}
+
 TEST_F(FormatTest, UnderstandsNewAndDelete) {
   verifyFormat("void f() {\n"
"  A *a = new A;\n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2282,9 +2282,11 @@
   }
 }
 if (Style.DerivePointerAlignment) {
-  Style.PointerAlignment = countVariableAlignments(AnnotatedLines) <= 0
-   ? FormatStyle::PAS_Left
-   : FormatStyle::PAS_Right;
+  const auto NetRightCount = countVariableAlignments(AnnotatedLines);
+  if (NetRightCount > 0)
+Style.PointerAlignment = FormatStyle::PAS_Right;
+  else if (NetRightCount < 0)
+Style.PointerAlignment = FormatStyle::PAS_Left;
   Style.ReferenceAlignment = FormatStyle::RAS_Pointer;
 }
 if (Style.Standard == FormatStyle::LS_Auto) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10781,6 +10781,33 @@
 format(Prefix + "int* x;", DerivePointerAlignment));
 }
 
+TEST_F(FormatTest, PointerAlignmentFallback) {
+  FormatStyle Style = getLLVMStyle();
+  Style.DerivePointerAlignment = true;
+
+  const StringRef Code("int* p;\n"
+   "int *q;\n"
+   "int * r;");
+
+  EXPECT_EQ(Style.PointerAlignment, FormatStyle::PAS_Right);
+  verifyFormat("int *p;\n"
+   "int *q;\n"
+   "int *r;",
+   Code, Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("int* p;\n"
+   "int* q;\n"
+   "int* r;",
+   Code, Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("int * p;\n"
+   "int * q;\n"
+   "int * r;",
+   Code, Style);
+}
+
 TEST_F(FormatTest, UnderstandsNewAndDelete) {
   verifyFormat("void f() {\n"
"  A *a = new A;\n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2282,9 +2282,11 @@
   }
 }
 if (Style.DerivePointerAlignment) {
-  Style.PointerAlignment = countVariableAlignments(AnnotatedLines) <= 0
-   ? FormatStyle::PAS_Left
-   : FormatStyle::PAS_Right;
+  const auto NetRightCount = countVariableAlignments(AnnotatedLines);
+  if (NetRightCount > 0)
+Style.PointerAlignment = FormatStyle::PAS_Right;
+  else if (NetRightCount < 0)
+Style.PointerAlignment = FormatStyle::PAS_Left;
   Style.ReferenceAlignment = FormatStyle::RAS_Pointer;
 }
 if (Style.Standard == FormatStyle::LS_Auto) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

[Fabian Keßler via Phabricator via cfe-commits]

Febbe added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-array-subscript-expression.cpp:18
   // CHECK-NOTES-C:(ptrdiff_t)( )
   // CHECK-NOTES-CXX:  static_cast( )
   // CHECK-NOTES-ALL: :[[@LINE-5]]:16: note: perform multiplication in a wider 
type

v1nh1shungry wrote:
> Actually I have tried adding
> 
> ```
> // CHECK-FIXES-C: return [(ptrdiff_t)(a * b)];
> // CHECK-FIXES-CXX: return [static_cast(a * b)];
> ```
> 
> under this line, but the test failed, and when I took a look at 
> `build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/implicit-widening-of-multiplication-result-array-subscript-expression.cpp.tmp.cpp`,
>  I found that these codes didn't get modified. And I took a look at other 
> files which have `CHECK-FIXES` lines, I found the codes in the corresponding 
> temporary files got fixed.
Maybe, because the Fixup is marked as Note


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

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

[PATCH] D141182: [mips][clang] Do not define __GCC_HAVE_SYNC_COMPARE_AND_SWAP macros for MIPS-I

[Fangrui Song via Phabricator via cfe-commits]

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

Looks great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141182

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

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

[Chuanqi Xu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/Driver.cpp:5736
+  C.getArgs().hasArg(options::OPT_fmodule_output) &&
+  C.getArgs().hasArg(options::OPT_o)) {
+SmallString<128> OutputPath;

dblaikie wrote:
> tahonermann wrote:
> > dblaikie wrote:
> > > ChuanqiXu wrote:
> > > > dblaikie wrote:
> > > > > Is there some way we can avoid this (`-fmodule-output  -o ...`) being 
> > > > > a special case? It'd be good if we could use a single common 
> > > > > implementation regardless of whether `-o` is used (ie: Figure out the 
> > > > > output file name (whether it's implicitly determined by clang, in the 
> > > > > absence of `-o`, or explicitly provided by the user through `-o`) and 
> > > > > then replace the suffix with `pcm`)?
> > > > I guess we can't do it or we can't do it easily. Otherwise the .pcm 
> > > > file will always lives in the same directory with the input file.
> > > > I guess we can't do it or we can't do it easily. Otherwise the .pcm 
> > > > file will always lives in the same directory with the input file.
> > > 
> > > I don't follow/understand. What I mean is, I'd like it, if possible, this 
> > > was implemented by something like "find the path for the .o file output, 
> > > then replace the extension with .pcm".
> > > 
> > > I worry atht code like this that recomputes something similar to the 
> > > object output path risks getting out of sync with the actual object path.
> > That is certainly a valid concern and I agree it would be better if the 
> > shared output path is computed exactly once. If that would require 
> > significant change, then tests to ensure consistent behavior would be the 
> > next best thing. I'm not sure what infrastructure is in place for 
> > validation of output file locations though.
> Well, I guess the Split DWARF file naming isn't fundamentally better - it 
> looks at the OPT_O argument directly too:
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/CommonArgs.cpp#L1250
> 
> Perhaps we could at least reuse this same logic - make it a reusable function 
> in some way? (for instance, it checks `-c` too, which seems relevant - we 
> wouldn't want to name the .pcm after the linked executable name, right?
> 
> Perhaps we could at least reuse this same logic - make it a reusable function 
> in some way? 

Done. It looks better now.

> (for instance, it checks -c too, which seems relevant - we wouldn't want to 
> name the .pcm after the linked executable name, right?

Oh, right. Although the previous conclusion is that if `-o` is specified, the 
.pcm file should be in the same dir with the output. But it is indeed weird 
that the .pcm file are related to the linked executable if we thought it 
deeper. 


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

https://reviews.llvm.org/D137058

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

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 488455.
ChuanqiXu added a comment.

Address comments:

- Extract the logic to compute the output path of `-fmodule-output` to a 
reusable function.


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

https://reviews.llvm.org/D137058

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/lit.local.cfg
  clang/test/Driver/module-output.cppm

Index: clang/test/Driver/module-output.cppm
===
--- /dev/null
+++ clang/test/Driver/module-output.cppm
@@ -0,0 +1,33 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// Tests that the .pcm file will be generated in the same directory with the specified
+// output and the name of the .pcm file should be the same with the input file.
+// RUN: %clang -std=c++20 %t/Hello.cppm -fmodule-output -c -o %t/output/Hello.o \
+// RUN:   -### 2>&1 | FileCheck %t/Hello.cppm
+//
+// Tests that the output file will be generated in the input directory if the output
+// file is not the corresponding object file.
+// RUN: %clang -std=c++20 %t/Hello.cppm %t/AnotherModule.cppm -fmodule-output -o \
+// RUN:   %t/output/a.out -### 2>&1 | FileCheck  %t/AnotherModule.cppm
+//
+// Tests that clang will reject the command line if it specifies -fmodule-output with
+// multiple archs.
+// RUN: %clang %t/Hello.cppm -fmodule-output -arch i386 -arch x86_64 -### -target \
+// RUN:   x86_64-apple-darwin 2>&1 | FileCheck %t/Hello.cppm -check-prefix=MULTIPLE-ARCH
+
+//--- Hello.cppm
+export module Hello;
+
+// CHECK: "-emit-module-interface" {{.*}}"-main-file-name" "Hello.cppm" {{.*}}"-o" "{{.*}}/output/Hello.pcm" "-x" "c++" "{{.*}}/Hello.cppm"
+// CHECK: "-emit-obj" {{.*}}"-main-file-name" "Hello.cppm" {{.*}}"-o" "{{.*}}/output/Hello.o" "-x" "pcm" "{{.*}}/output/Hello.pcm"
+
+// MULTIPLE-ARCH: option '-fmodule-output' can't be used with multiple arch options
+
+//--- AnotherModule.cppm
+export module AnotherModule;
+// CHECK: "-emit-module-interface" {{.*}}"-main-file-name" "Hello.cppm" {{.*}}"-o" "{{.*}}/Hello.pcm" "-x" "c++" "{{.*}}/Hello.cppm"
+// CHECK: "-emit-obj" {{.*}}"-main-file-name" "Hello.cppm" {{.*}}"-o" "{{.*}}/Hello-{{.*}}.o" "-x" "pcm" "{{.*}}/Hello.pcm"
+// CHECK: "-emit-module-interface" {{.*}}"-main-file-name" "AnotherModule.cppm" {{.*}}"-o" "{{.*}}/AnotherModule.pcm" "-x" "c++" "{{.*}}/AnotherModule.cppm"
+// CHECK: "-emit-obj" {{.*}}"-main-file-name" "AnotherModule.cppm" {{.*}}"-o" "{{.*}}/AnotherModule-{{.*}}.o" "-x" "pcm" "{{.*}}/AnotherModule.pcm"
Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,6 +1,6 @@
 from lit.llvm import llvm_config
 
-config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
+config.suffixes = ['.c', '.cpp', '.cppm', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
'.cu', '.rs', '.cl', '.clcpp', '.hip', '.hipi', '.hlsl']
 config.substitutions = list(config.substitutions)
 config.substitutions.insert(0,
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5553,6 +5553,30 @@
   return C.addTempFile(C.getArgs().MakeArgString(TmpName));
 }
 
+// Calculate the output path of the module file when compiling a module unit
+// with the `-fmodule-output` option specified. The behavior is:
+// - If the output object file of the module unit is specified, the output path
+//   of the module file should be the same with the output object file except
+//   the corresponding suffix. This requires both `-o` and `-c` are specified.
+// - Otherwise, the output path of the module file will be the same with the
+//   input with the corresponding suffix.
+static const char *GetModuleOutputPath(Compilation , const JobAction ,
+   const char *BaseInput) {
+  assert(isa(JA) && JA.getType() == types::TY_ModuleFile &&
+ C.getArgs().hasArg(options::OPT_fmodule_output));
+
+  SmallString<64> OutputPath;
+  Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o);
+  if (FinalOutput && C.getArgs().hasArg(options::OPT_c))
+OutputPath = FinalOutput->getValue();
+  else
+OutputPath = BaseInput;
+
+  const char *Extension = types::getTypeTempSuffix(JA.getType());
+  llvm::sys::path::replace_extension(OutputPath, Extension);
+  return C.addResultFile(C.getArgs().MakeArgString(OutputPath.c_str()), );
+}
+
 const char *Driver::GetNamedOutputPath(Compilation , const JobAction ,
const char *BaseInput,
StringRef OrigBoundArch, bool AtTopLevel,
@@ -5609,6 +5633,16 @@
 );
   }
 
+  if (MultipleArchs && 

[PATCH] D141545: [OpenMP] Implement `omp_get_mapped_ptr`

[Shilei Tian 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 rG6e18277a5118: [OpenMP] Implement `omp_get_mapped_ptr` 
(authored by tianshilei1992).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141545

Files:
  clang/docs/OpenMPSupport.rst
  openmp/libomptarget/src/api.cpp
  openmp/libomptarget/src/exports
  openmp/libomptarget/test/api/omp_get_mapped_ptr.c

Index: openmp/libomptarget/test/api/omp_get_mapped_ptr.c
===
--- /dev/null
+++ openmp/libomptarget/test/api/omp_get_mapped_ptr.c
@@ -0,0 +1,39 @@
+// RUN: %libomptarget-compile-and-run-generic
+
+#include 
+#include 
+#include 
+
+#define N 1024
+#define OFFSET 16
+
+int main(int argc, char *argv[]) {
+  int *host_data = (int *)malloc(sizeof(int) * N);
+  void *device_ptr = omp_get_mapped_ptr(host_data, 0);
+
+  assert(device_ptr == NULL && "the pointer should not be mapped right now");
+
+#pragma omp target enter data map(to: host_data[:N])
+
+  device_ptr = omp_get_mapped_ptr(host_data, 0);
+
+  assert(device_ptr && "the pointer should be mapped now");
+
+  void *ptr = NULL;
+
+#pragma omp target map(from: ptr)
+  { ptr = host_data; }
+
+  assert(ptr == device_ptr && "wrong pointer mapping");
+
+  device_ptr = omp_get_mapped_ptr(host_data + OFFSET, 0);
+
+  assert(device_ptr && "the pointer with offset should be mapped");
+
+#pragma omp target map(from: ptr)
+  { ptr = host_data + OFFSET; }
+
+  assert(ptr == device_ptr && "wrong pointer mapping");
+
+  return 0;
+}
Index: openmp/libomptarget/src/exports
===
--- openmp/libomptarget/src/exports
+++ openmp/libomptarget/src/exports
@@ -31,6 +31,7 @@
 __tgt_push_mapper_component;
 __kmpc_push_target_tripcount;
 __kmpc_push_target_tripcount_mapper;
+omp_get_mapped_ptr;
 omp_get_num_devices;
 omp_get_device_num;
 omp_get_initial_device;
Index: openmp/libomptarget/src/api.cpp
===
--- openmp/libomptarget/src/api.cpp
+++ openmp/libomptarget/src/api.cpp
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 EXTERN int omp_get_num_devices(void) {
   TIMESCOPE();
@@ -318,3 +319,52 @@
   DP("omp_target_disassociate_ptr returns %d\n", Rc);
   return Rc;
 }
+
+EXTERN void *omp_get_mapped_ptr(const void *Ptr, int DeviceNum) {
+  TIMESCOPE();
+  DP("Call to omp_get_mapped_ptr with ptr " DPxMOD ", device_num %d.\n",
+ DPxPTR(Ptr), DeviceNum);
+
+  if (!Ptr) {
+REPORT("Call to omp_get_mapped_ptr with nullptr.\n");
+return nullptr;
+  }
+
+  if (DeviceNum == omp_get_initial_device()) {
+REPORT("Device %d is initial device, returning Ptr " DPxMOD ".\n",
+   DeviceNum, DPxPTR(Ptr));
+return const_cast(Ptr);
+  }
+
+  int DevicesSize = omp_get_initial_device();
+  {
+std::lock_guard LG(PM->RTLsMtx);
+DevicesSize = PM->Devices.size();
+  }
+  if (DevicesSize <= DeviceNum) {
+DP("DeviceNum %d is invalid, returning nullptr.\n", DeviceNum);
+return nullptr;
+  }
+
+  if (!deviceIsReady(DeviceNum)) {
+REPORT("Device %d is not ready, returning nullptr.\n", DeviceNum);
+return nullptr;
+  }
+
+  bool IsLast = false;
+  bool IsHostPtr = false;
+  auto  = *PM->Devices[DeviceNum];
+  TargetPointerResultTy TPR =
+  Device.getTgtPtrBegin(const_cast(Ptr), 1, IsLast,
+/*UpdateRefCount=*/false,
+/*UseHoldRefCount=*/false, IsHostPtr);
+  if (!TPR.isPresent()) {
+DP("Ptr " DPxMOD "is not present on device %d, returning nullptr.\n",
+   DPxPTR(Ptr), DeviceNum);
+return nullptr;
+  }
+
+  DP("omp_get_mapped_ptr returns " DPxMOD ".\n", DPxPTR(TPR.TargetPointer));
+
+  return TPR.TargetPointer;
+}
Index: clang/docs/OpenMPSupport.rst
===
--- clang/docs/OpenMPSupport.rst
+++ clang/docs/OpenMPSupport.rst
@@ -111,7 +111,7 @@
 
 The following table provides a quick overview over various OpenMP 5.0 features
 and their implementation status. Please post on the
-`Discourse forums (Runtimes - OpenMP category)`_ for more 
+`Discourse forums (Runtimes - OpenMP category)`_ for more
 information or if you want to help with the
 implementation.
 
@@ -257,8 +257,8 @@
 
 The following table provides a quick overview over various OpenMP 5.1 features
 and their implementation status, as defined in the technical report 8 (TR8).
-Please post on the 
-`Discourse forums (Runtimes - OpenMP category)`_ for more 
+Please post on the
+`Discourse forums (Runtimes - OpenMP category)`_ for more
 information or if you want to help with the
 implementation.
 
@@ -283,7 +283,7 @@
 

[clang] 6e18277 - [OpenMP] Implement `omp_get_mapped_ptr`

[Shilei Tian via cfe-commits]

Author: Shilei Tian
Date: 2023-01-11T22:05:42-05:00
New Revision: 6e18277a51187ce8e861cdf0ab1395235e5b83d4

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

LOG: [OpenMP] Implement `omp_get_mapped_ptr`

This patch implements the function `omp_get_mapped_ptr`.

Fix #59945.

Reviewed By: jdoerfert

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

Added: 
openmp/libomptarget/test/api/omp_get_mapped_ptr.c

Modified: 
clang/docs/OpenMPSupport.rst
openmp/libomptarget/src/api.cpp
openmp/libomptarget/src/exports

Removed: 




diff  --git a/clang/docs/OpenMPSupport.rst b/clang/docs/OpenMPSupport.rst
index dca1486f9967f..16cb50afa1a13 100644
--- a/clang/docs/OpenMPSupport.rst
+++ b/clang/docs/OpenMPSupport.rst
@@ -111,7 +111,7 @@ OpenMP 5.0 Implementation Details
 
 The following table provides a quick overview over various OpenMP 5.0 features
 and their implementation status. Please post on the
-`Discourse forums (Runtimes - OpenMP category)`_ for more 
+`Discourse forums (Runtimes - OpenMP category)`_ for more
 information or if you want to help with the
 implementation.
 
@@ -257,8 +257,8 @@ OpenMP 5.1 Implementation Details
 
 The following table provides a quick overview over various OpenMP 5.1 features
 and their implementation status, as defined in the technical report 8 (TR8).
-Please post on the 
-`Discourse forums (Runtimes - OpenMP category)`_ for more 
+Please post on the
+`Discourse forums (Runtimes - OpenMP category)`_ for more
 information or if you want to help with the
 implementation.
 
@@ -283,7 +283,7 @@ implementation.
 
+--+--+--+---+
 | device   | omp_target_is_accessible routine  
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+
-| device   | omp_get_mapped_ptr routine
   | :none:`unclaimed`| 
  |
+| device   | omp_get_mapped_ptr routine
   | :none:`done` | 
  |
 
+--+--+--+---+
 | device   | new async target memory copy routines 
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+
@@ -365,12 +365,12 @@ implementation.
 OpenMP Extensions
 =
 
-The following table provides a quick overview over various OpenMP 
+The following table provides a quick overview over various OpenMP
 extensions and their implementation status.  These extensions are not
 currently defined by any standard, so links to associated LLVM
 documentation are provided.  As these extensions mature, they will be
 considered for standardization. Please post on the
-`Discourse forums (Runtimes - OpenMP category)`_ to provide feedback. 
+`Discourse forums (Runtimes - OpenMP category)`_ to provide feedback.
 
 
+--+---+--++
 |Category  | Feature   
| Status   | Reviews
|

diff  --git a/openmp/libomptarget/src/api.cpp b/openmp/libomptarget/src/api.cpp
index f408449f01134..f96a2be2146e8 100644
--- a/openmp/libomptarget/src/api.cpp
+++ b/openmp/libomptarget/src/api.cpp
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 EXTERN int omp_get_num_devices(void) {
   TIMESCOPE();
@@ -318,3 +319,52 @@ EXTERN int omp_target_disassociate_ptr(const void 
*HostPtr, int DeviceNum) {
   DP("omp_target_disassociate_ptr returns %d\n", Rc);
   return Rc;
 }
+
+EXTERN void *omp_get_mapped_ptr(const void *Ptr, int DeviceNum) {
+  TIMESCOPE();
+  DP("Call to 

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D138958#4045567 , @efriedma wrote:

> From an IR semantics standpoint, I'm not sure the `memory(none)` marking is 
> right if the function throws an exception.

Then `memory(read)` would be wrong, either.
A bit of a hot take: it's a Schrodinger's exception in quantum state.
What i mean by that, is that e.g. `pure` attribute is documented as

  and will not cause any observable side-effects other than
  returning a value. They may read memory, but can not modify memory in a way
  that would either affect observable state or their outcome on subsequent runs.

So if it does unwind, that's fine, but if you *observed* that (not by catching 
an exception),
then you're in UB lands, because *you*, not the function, violated it's 
contract.

That might be too hot of a take, but it does seem like the EH internal details 
aren't really *that* observable...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138958

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

[PATCH] D141561: [clang] True `noexcept` (`-fstrict-noexcept` language dialect)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: aaron.ballman, rjmccall, efriedma, jcranmer, 
jyknight, rsmith.
lebedev.ri added a project: LLVM.
Herald added subscribers: StephenFan, dschuff.
Herald added a project: All.
lebedev.ri requested review of this revision.
Herald added subscribers: MaskRay, aheejin.
Herald added a project: clang.

This tentatively implements the following RFC:
https://discourse.llvm.org/t/rfc-clang-true-noexcept-aka-defaults-are-often-wrong-hardcoded-defaults-are-always-wrong/67629

The idea is that when the opt-in is specified,
we turn all EH Terminate scopes into EH UB scopes,
and don't emit any `std::terminate()` calls,
which is the user-facing change.

This probably needs better docs,
and might be missing some pieces.

We might be able to do more to omit more destructor calls.

This is missing UBSan bits, those will be in D137381 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141561

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGCleanup.h
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCXX/exception-escape-as-ub-cleanups.cpp
  clang/test/CodeGenCXX/exception-escape.cpp

Index: clang/test/CodeGenCXX/exception-escape.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/exception-escape.cpp
@@ -0,0 +1,632 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --check-attributes --check-globals
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-linux-gnu -fcxx-exceptions| FileCheck %s --check-prefixes=CHECK-ALL,CHECK-NO-EXCEPTIONS
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-linux-gnu -fcxx-exceptions -fexceptions   | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-EXCEPTIONS,CHECK-NORMAL-NOEXCEPT
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-linux-gnu -fcxx-exceptions -fexceptions -fstrict-noexcept | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-EXCEPTIONS,CHECK-STRICT-NOEXCEPT
+
+void will_throw(int line = __builtin_LINE());
+void might_throw(int line = __builtin_LINE());
+void will_not_throw(int line = __builtin_LINE()) noexcept;
+
+//.
+// CHECK-ALL: @_ZTIi = external constant ptr
+//.
+// CHECK-NO-EXCEPTIONS: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-NO-EXCEPTIONS-LABEL: define {{[^@]+}}@_Z45exception_escape_is_program_termination_or_ubi
+// CHECK-NO-EXCEPTIONS-SAME: (i32 noundef [[X:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-NO-EXCEPTIONS-NEXT:  entry:
+// CHECK-NO-EXCEPTIONS-NEXT:[[X_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NO-EXCEPTIONS-NEXT:store i32 [[X]], ptr [[X_ADDR]], align 4
+// CHECK-NO-EXCEPTIONS-NEXT:[[TMP0:%.*]] = load i32, ptr [[X_ADDR]], align 4
+// CHECK-NO-EXCEPTIONS-NEXT:[[CMP:%.*]] = icmp eq i32 [[TMP0]], 2
+// CHECK-NO-EXCEPTIONS-NEXT:br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+// CHECK-NO-EXCEPTIONS:   if.then:
+// CHECK-NO-EXCEPTIONS-NEXT:call void @_Z10will_throwi(i32 noundef 100)
+// CHECK-NO-EXCEPTIONS-NEXT:br label [[IF_END]]
+// CHECK-NO-EXCEPTIONS:   if.end:
+// CHECK-NO-EXCEPTIONS-NEXT:call void @_Z14will_not_throwi(i32 noundef 200) #[[ATTR3:[0-9]+]]
+// CHECK-NO-EXCEPTIONS-NEXT:[[TMP1:%.*]] = load i32, ptr [[X_ADDR]], align 4
+// CHECK-NO-EXCEPTIONS-NEXT:[[CMP1:%.*]] = icmp eq i32 [[TMP1]], 3
+// CHECK-NO-EXCEPTIONS-NEXT:br i1 [[CMP1]], label [[IF_THEN2:%.*]], label [[IF_END3:%.*]]
+// CHECK-NO-EXCEPTIONS:   if.then2:
+// CHECK-NO-EXCEPTIONS-NEXT:call void @_Z10will_throwi(i32 noundef 300)
+// CHECK-NO-EXCEPTIONS-NEXT:br label [[IF_END3]]
+// CHECK-NO-EXCEPTIONS:   if.end3:
+// CHECK-NO-EXCEPTIONS-NEXT:[[TMP2:%.*]] = load i32, ptr [[X_ADDR]], align 4
+// CHECK-NO-EXCEPTIONS-NEXT:[[CMP4:%.*]] = icmp eq i32 [[TMP2]], 4
+// CHECK-NO-EXCEPTIONS-NEXT:br i1 [[CMP4]], label [[IF_THEN5:%.*]], label [[IF_END6:%.*]]
+// CHECK-NO-EXCEPTIONS:   if.then5:
+// CHECK-NO-EXCEPTIONS-NEXT:call void @_Z14will_not_throwi(i32 noundef 400) #[[ATTR3]]
+// CHECK-NO-EXCEPTIONS-NEXT:call void @_Z10will_throwi(i32 noundef 500)
+// CHECK-NO-EXCEPTIONS-NEXT:br label [[IF_END6]]
+// CHECK-NO-EXCEPTIONS:   if.end6:
+// CHECK-NO-EXCEPTIONS-NEXT:[[TMP3:%.*]] = load i32, ptr [[X_ADDR]], align 4
+// CHECK-NO-EXCEPTIONS-NEXT:[[CMP7:%.*]] = icmp eq i32 [[TMP3]], 5
+// CHECK-NO-EXCEPTIONS-NEXT:br i1 [[CMP7]], label [[IF_THEN8:%.*]], label [[IF_END9:%.*]]
+// CHECK-NO-EXCEPTIONS:   if.then8:
+// CHECK-NO-EXCEPTIONS-NEXT:call void @_Z10will_throwi(i32 noundef 900)
+// 

[PATCH] D140867: [C++20] [Modules] Don't generate global ctors/dtors for variables which are available externally

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D140867#4043763 , @Jake-Egan wrote:

> In D140867#4042405 , @ChuanqiXu 
> wrote:
>
>> In D140867#4042351 , @Jake-Egan 
>> wrote:
>>
 Would you like to take a double look?
>>>
>>> Yes, it still fails on the bot and on my local machine. I've added an XFAIL 
>>> to the test for now just to get the AIX bot green.
>>
>> Weird. Maybe you can find a method by adding `REQUIRES` or `NOT-SUPPORTED` 
>> and test it locally. If it goes well then you can upstream the change.
>
> It seems that it's because the test fails when running on AIX - not just when 
> it's being targeted.
>
>   // REQUIRES: x86-registered-target,aarch64-registered-target
>
> The above doesn't stop the test from being run on AIX. So, I can add:
>
>   // UNSUPPORTED: system-aix
>
> But do you want to keep the REQUIRES for other targets?

The `REQUIRES` are added for `AIX`, so maybe it is ok to abandon them 
temporarily and add them back once we find other similar failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140867

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

[PATCH] D129570: [clang-tidy] Add new clang-tidy check to find implicit conversions from enum to integer.

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 updated this revision to Diff 488445.
pfultz2 added a comment.

Merge from main.


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

https://reviews.llvm.org/D129570

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-int.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-int.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-int.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-int.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy -std=c++11-or-later --fix-notes %s bugprone-enum-to-int %t
+
+enum A { 
+  e1,
+  e2 
+};
+
+enum B : unsigned int {
+  e3,
+  e4
+};
+
+struct bar {
+  bar(int);
+};
+void foo(int i);
+void foo_unsigned(unsigned int i);
+void f1() {
+  foo(e1);
+  // CHECK-NOTES: :[[@LINE-1]]:7: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:7: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: foo(static_cast(e1));
+}
+void f2() {
+  foo(static_cast(e2));
+}
+void f3() {
+  int i = e1;
+  foo(i);
+}
+void f4() {
+  bar a(e1);
+  // CHECK-NOTES: :[[@LINE-1]]:9: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:9: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: bar a(static_cast(e1));
+}
+void f5() {
+  auto a = bar{e1};
+  // CHECK-NOTES: :[[@LINE-1]]:16: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:16: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: auto a = bar{static_cast(e1)};
+}
+int f6() {
+  return e1;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: return static_cast(e1);
+}
+void f7() {
+  foo_unsigned(e1);
+  // CHECK-NOTES: :[[@LINE-1]]:16: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:16: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: foo_unsigned(static_cast(e1));
+}
+void f8() {
+  foo(e4);
+  // CHECK-NOTES: :[[@LINE-1]]:7: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:7: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: foo(static_cast(e4));
+}
+void f9() {
+  foo_unsigned(e4);
+  // CHECK-NOTES: :[[@LINE-1]]:16: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:16: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: foo_unsigned(static_cast(e4));
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -86,6 +86,7 @@
`bugprone-dangling-handle `_,
`bugprone-dynamic-static-initializers `_,
`bugprone-easily-swappable-parameters `_,
+   `bugprone-enum-to-int `_,
`bugprone-exception-escape `_,
`bugprone-fold-init-type `_,
`bugprone-forward-declaration-namespace `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-int.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-int.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - bugprone-enum-to-int
+
+bugprone-enum-to-int
+
+
+This check diagnoses instances where an enum is implicitly converted to an
+integer. In C++11, enums can be defined as ``enum class`` which will prevent
+such implicit conversion, however, ``enum`` provides no such guarantees to
+prevent bugs. There can be many reasons why ``enum`` cannot be replaced with
+``enum class`` such as compatibility with C or legacy libraries.
+
+This check will diagnose similiar implicit conversions whne using ``enum`` to
+find the same class of bugs. Currently it will only warn on function or
+constructor calls, or return statements as such conversions are not always
+clear to the user, but this could be expanded in the future.
+
+Examples:
+
+.. code-block:: c++
+
+void foo(int i);
+void f() {
+foo(e1); // e1 is implictly converted to an int
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst

[PATCH] D141550: [CompilerRT] Remove ubsan static runtime on Apple

[Dan Liew via Phabricator via cfe-commits]

delcypher accepted this revision.
delcypher 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/D141550/new/

https://reviews.llvm.org/D141550

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

[PATCH] D140455: [Clang] Diagnose undefined behavior in a constant expression while evaluating a compound assignment with remainder as operand

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 488441.
shafik added a comment.

- Moving back HandleOverflowResult to before the switch statement to avoid 
"jump bypasses variable initialization" error.


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

https://reviews.llvm.org/D140455

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/CXX/expr/expr.const/p2-0x.cpp


Index: clang/test/CXX/expr/expr.const/p2-0x.cpp
===
--- clang/test/CXX/expr/expr.const/p2-0x.cpp
+++ clang/test/CXX/expr/expr.const/p2-0x.cpp
@@ -298,6 +298,15 @@
 static_assert(isinf(f6), "");
 static_assert(isinf(f9), "");
   }
+
+#if __cplusplus >= 201703L
+namespace CompoundAssignment {
+constexpr int rem() { // expected-error {{constexpr function never produces a 
constant expression}}
+int x = ~__INT_MAX__;
+return x%=-1; // cxx20-note {{value 2147483648 is outside the range of 
representable values of type 'int'}}
+}
+}
+#endif
 }
 
 // - a lambda-expression (5.1.2);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2751,6 +2751,7 @@
 static bool handleIntIntBinOp(EvalInfo , const Expr *E, const APSInt ,
   BinaryOperatorKind Opcode, APSInt RHS,
   APSInt ) {
+  bool HandleOverflowResult = true;
   switch (Opcode) {
   default:
 Info.FFDiag(E);
@@ -2773,14 +2774,14 @@
   Info.FFDiag(E, diag::note_expr_divide_by_zero);
   return false;
 }
-Result = (Opcode == BO_Rem ? LHS % RHS : LHS / RHS);
 // Check for overflow case: INT_MIN / -1 or INT_MIN % -1. APSInt supports
 // this operation and gives the two's complement result.
 if (RHS.isNegative() && RHS.isAllOnes() && LHS.isSigned() &&
 LHS.isMinSignedValue())
-  return HandleOverflow(Info, E, -LHS.extend(LHS.getBitWidth() + 1),
-E->getType());
-return true;
+  HandleOverflowResult = HandleOverflow(
+  Info, E, -LHS.extend(LHS.getBitWidth() + 1), E->getType());
+Result = (Opcode == BO_Rem ? LHS % RHS : LHS / RHS);
+return HandleOverflowResult;
   case BO_Shl: {
 if (Info.getLangOpts().OpenCL)
   // OpenCL 6.3j: shift values are effectively % word size of LHS.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -447,6 +447,8 @@
   ``#pragma clang __debug sloc_usage`` can also be used to request this report.
 - Clang no longer permits the keyword 'bool' in a concept declaration as a
   concepts-ts compatibility extension.
+- Clang now diagnoses overflow undefined behavior in a constant expression 
while
+  evaluating a compound assignment with remainder as operand.
 
 Non-comprehensive list of changes in this release
 -


Index: clang/test/CXX/expr/expr.const/p2-0x.cpp
===
--- clang/test/CXX/expr/expr.const/p2-0x.cpp
+++ clang/test/CXX/expr/expr.const/p2-0x.cpp
@@ -298,6 +298,15 @@
 static_assert(isinf(f6), "");
 static_assert(isinf(f9), "");
   }
+
+#if __cplusplus >= 201703L
+namespace CompoundAssignment {
+constexpr int rem() { // expected-error {{constexpr function never produces a constant expression}}
+int x = ~__INT_MAX__;
+return x%=-1; // cxx20-note {{value 2147483648 is outside the range of representable values of type 'int'}}
+}
+}
+#endif
 }
 
 // - a lambda-expression (5.1.2);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2751,6 +2751,7 @@
 static bool handleIntIntBinOp(EvalInfo , const Expr *E, const APSInt ,
   BinaryOperatorKind Opcode, APSInt RHS,
   APSInt ) {
+  bool HandleOverflowResult = true;
   switch (Opcode) {
   default:
 Info.FFDiag(E);
@@ -2773,14 +2774,14 @@
   Info.FFDiag(E, diag::note_expr_divide_by_zero);
   return false;
 }
-Result = (Opcode == BO_Rem ? LHS % RHS : LHS / RHS);
 // Check for overflow case: INT_MIN / -1 or INT_MIN % -1. APSInt supports
 // this operation and gives the two's complement result.
 if (RHS.isNegative() && RHS.isAllOnes() && LHS.isSigned() &&
 LHS.isMinSignedValue())
-  return HandleOverflow(Info, E, -LHS.extend(LHS.getBitWidth() + 1),
-E->getType());
-return true;
+  HandleOverflowResult = HandleOverflow(
+  Info, E, -LHS.extend(LHS.getBitWidth() + 1), E->getType());
+Result = (Opcode == BO_Rem ? LHS % RHS : LHS / RHS);
+return HandleOverflowResult;
   case BO_Shl: {
 if (Info.getLangOpts().OpenCL)
  

[PATCH] D129570: [clang-tidy] Add new clang-tidy check to find implicit conversions from enum to integer.

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 updated this revision to Diff 488440.
pfultz2 added a comment.

Improve null checking, use the correct type in the fixit, and updated the tests.


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

https://reviews.llvm.org/D129570

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-int.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-int.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-int.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-int.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy -std=c++11-or-later --fix-notes %s bugprone-enum-to-int %t
+
+enum A { 
+  e1,
+  e2 
+};
+
+enum B : unsigned int {
+  e3,
+  e4
+};
+
+struct bar {
+  bar(int);
+};
+void foo(int i);
+void foo_unsigned(unsigned int i);
+void f1() {
+  foo(e1);
+  // CHECK-NOTES: :[[@LINE-1]]:7: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:7: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: foo(static_cast(e1));
+}
+void f2() {
+  foo(static_cast(e2));
+}
+void f3() {
+  int i = e1;
+  foo(i);
+}
+void f4() {
+  bar a(e1);
+  // CHECK-NOTES: :[[@LINE-1]]:9: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:9: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: bar a(static_cast(e1));
+}
+void f5() {
+  auto a = bar{e1};
+  // CHECK-NOTES: :[[@LINE-1]]:16: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:16: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: auto a = bar{static_cast(e1)};
+}
+int f6() {
+  return e1;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: return static_cast(e1);
+}
+void f7() {
+  foo_unsigned(e1);
+  // CHECK-NOTES: :[[@LINE-1]]:16: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:16: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: foo_unsigned(static_cast(e1));
+}
+void f8() {
+  foo(e4);
+  // CHECK-NOTES: :[[@LINE-1]]:7: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:7: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: foo(static_cast(e4));
+}
+void f9() {
+  foo_unsigned(e4);
+  // CHECK-NOTES: :[[@LINE-1]]:16: warning: enum is implictly converted to an integral [bugprone-enum-to-int]
+  // CHECK-NOTES: :[[@LINE-2]]:16: note: insert an explicit cast
+  // CHECK-NOTES: static_cast( )
+  // CHECK-FIXES: foo_unsigned(static_cast(e4));
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -86,6 +86,7 @@
`bugprone-dangling-handle `_,
`bugprone-dynamic-static-initializers `_,
`bugprone-easily-swappable-parameters `_,
+   `bugprone-enum-to-int `_,
`bugprone-exception-escape `_,
`bugprone-fold-init-type `_,
`bugprone-forward-declaration-namespace `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-int.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-int.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - bugprone-enum-to-int
+
+bugprone-enum-to-int
+
+
+This check diagnoses instances where an enum is implicitly converted to an
+integer. In C++11, enums can be defined as ``enum class`` which will prevent
+such implicit conversion, however, ``enum`` provides no such guarantees to
+prevent bugs. There can be many reasons why ``enum`` cannot be replaced with
+``enum class`` such as compatibility with C or legacy libraries.
+
+This check will diagnose similiar implicit conversions whne using ``enum`` to
+find the same class of bugs. Currently it will only warn on function or
+constructor calls, or return statements as such conversions are not always
+clear to the user, but this could be expanded in the future.
+
+Examples:
+
+.. code-block:: c++
+
+void foo(int i);
+void f() {
+foo(e1); // e1 is implictly converted to an int
+}
Index: 

[clang] e97caa9 - Fixed Clang::Driver 'netbsd.c' test on Windows/Cross builders. NFC.

[Vladimir Vereschaka via cfe-commits]

Author: Vladimir Vereschaka
Date: 2023-01-11T17:30:23-08:00
New Revision: e97caa9a2855bd928d1d4ff5503262baa92b3b7f

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

LOG: Fixed Clang::Driver 'netbsd.c' test on Windows/Cross builders. NFC.

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

Added: 


Modified: 
clang/test/Driver/netbsd.c

Removed: 




diff  --git a/clang/test/Driver/netbsd.c b/clang/test/Driver/netbsd.c
index 5d76aac525d9..3fefaeb100e2 100644
--- a/clang/test/Driver/netbsd.c
+++ b/clang/test/Driver/netbsd.c
@@ -482,5 +482,5 @@
 // RUN: %clang -### %s --target=x86_64-unknown-netbsd -r 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
 // DRIVER-PASS-INCLUDES:  "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
-// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" 
"[[RESOURCE]]{{/|}}include"
+// DRIVER-PASS-INCLUDES-SAME: "-internal-isystem" 
"[[RESOURCE]]{{/|}}include"
 // DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-externc-isystem" 
"{{.*}}/usr/include"



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

[PATCH] D141240: [SVE][Builtins] Add metadata to intrinsic calls for builtins that don't define the result of inactive lanes.

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

In D141240#4035438 , @sdesmalen wrote:

> Using metadata seems sensible, but did you also identify any downsides? I 
> could imagine that we'd need to manually propagate metadata to any nodes 
> after we do a combine (which can't be blindly copied?), e.g. add + mul -> 
> mla, this new intrinsic would also need the metadata.

I don't really see manually propagation as a downside because it's not 
functionally required but rather advantageous to maximise optimisation 
opportunities.  The downside is the opposite in that any transformation that 
wants to rely on the inactive lanes being defined as before this patch will now 
need to check for the presence of (or rather lack of) the new metadata before 
blindly reusing the result of an existing SVE intrinsic call.  The 
transformation can still reuse the call it must just first discard the metadata.

> For intrinsics that don't have a directly corresponding (unpredicated) LLVM 
> IR instruction, is there still a way to use this information in SelectionDAG?

Truth be told I'm not entire sure how this will play out.  I'm not sure whether 
it's better to use the information within the IR as I'm doing in this patch or 
whether this should be used solely when lowering IR to DAG.  So it's really an 
experiment to see what sticks while proving a route to fix some of the issues 
we've already observed with how we represent the X forms.

Predicated->unpredicted aside another use for encoding undefiness is that it 
helps with things the FMAs where we can use FMAD if that better suits register 
allocation much like we do for stock IR.

>> the select instruction itself has strict rules relating to poison that 
>> hampered the intent of this change
>
> For my understanding, can you elaborate what these strict rules regarding 
> poison are that hamper such a change, and what it was that you tried?

The LangRef states the transformation "select P, A, undef ==> A" is only valid 
when you can prove the inactive lanes of "A" do not contain poison. I'm unsure 
if this is a true blocker or a mere inconvenience because to maintain the 
maximum amount of information we likely don't want to remove the selects 
anyway.  I went down this path by creating an SVE undef intrinsic, which 
nothing knows about and thus will be left alone.  The problem is that it 
massively polluted the IR and I was worried it'll make it harder to 
spot/implement the typical combines. For sure the existing combines will need 
to be changed because they'll not know to look through the new selects.

There is the option to change the clang builtin lowering to provide finer 
control over which builtins emit these selects, but that just means more 
changes (updates to existing instcombines) each time we decide a builtin is 
worth the extra select.

I'll keep experimenting but as I mention within the in code comment, the likely 
best solution is to have dedicate intrinsics with this being the least 
intrusive hack.

Perhaps the key word there is "hack" :) I'll investigate the dedicate 
intrinsics route because perhaps we only require a handful to get the majority 
of the benefit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141240

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

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 added inline comments.



Comment at: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h:46
+/// The text indicating that the user needs to provide input there:
+constexpr static const char *const UserFillPlaceHolder = "...";
 } // end namespace clang

jkorous wrote:
> jkorous wrote:
> > Should we rather pick something that is syntactically incorrect in C++ in 
> > order to prevent accidental silent corruption of the sources?
> > FWIW Xcode uses `<#placeholder#>` syntax.
> Correction - it is not Xcode, it is clang itself.
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/CodeCompleteConsumer.cpp#L323
It looks like we could put different messages in between `<#` and `#>` to hint 
users in different situations.   I'll make this member a function then.


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

https://reviews.llvm.org/D139737

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

[PATCH] D141558: [MemProf] Collect access density statistics during profiling

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson created this revision.
tejohnson added reviewers: snehasish, davidxl.
Herald added subscribers: Enna1, wenlei.
Herald added a project: All.
tejohnson requested review of this revision.
Herald added projects: clang, Sanitizers, LLVM.
Herald added subscribers: Sanitizers, cfe-commits.

Track min/max/avg access density (accesses per byte and accesses per
byte per lifetime second) metrics directly during profiling. This allows
more accurate use of these metrics in profile analysis and use, instead
of trying to compute them from already aggregated data in the profile.

This required regenerating some of the raw profile and executable inputs
for a few tests. While here, make the llvm-profdata memprof tests more
resilient to differences in things like memory mapping, timestamps and
cpu ids to make future test updates easier.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141558

Files:
  clang/test/CodeGen/Inputs/memprof.memprofraw
  compiler-rt/include/profile/MIBEntryDef.inc
  compiler-rt/include/profile/MemProfData.inc
  llvm/include/llvm/ProfileData/MIBEntryDef.inc
  llvm/include/llvm/ProfileData/MemProfData.inc
  llvm/test/Transforms/PGOProfile/Inputs/memprof.exe
  llvm/test/Transforms/PGOProfile/Inputs/memprof.memprofraw
  llvm/test/tools/llvm-profdata/Inputs/basic.memprofexe
  llvm/test/tools/llvm-profdata/Inputs/basic.memprofraw
  llvm/test/tools/llvm-profdata/Inputs/inline.memprofexe
  llvm/test/tools/llvm-profdata/Inputs/inline.memprofraw
  llvm/test/tools/llvm-profdata/Inputs/multi.memprofexe
  llvm/test/tools/llvm-profdata/Inputs/multi.memprofraw
  llvm/test/tools/llvm-profdata/memprof-basic.test
  llvm/test/tools/llvm-profdata/memprof-inline.test
  llvm/test/tools/llvm-profdata/memprof-multi.test

Index: llvm/test/tools/llvm-profdata/memprof-multi.test
===
--- llvm/test/tools/llvm-profdata/memprof-multi.test
+++ llvm/test/tools/llvm-profdata/memprof-multi.test
@@ -41,7 +41,7 @@
 CHECK:  MemprofProfile:
 CHECK-NEXT:  Summary:
 CHECK-NEXT:Version: 1
-CHECK-NEXT:NumSegments: 9
+CHECK-NEXT:NumSegments: 7
 CHECK-NEXT:NumMibInfo: 2
 CHECK-NEXT:NumAllocFunctions: 1
 CHECK-NEXT:NumStackOffsets: 2
Index: llvm/test/tools/llvm-profdata/memprof-inline.test
===
--- llvm/test/tools/llvm-profdata/memprof-inline.test
+++ llvm/test/tools/llvm-profdata/memprof-inline.test
@@ -41,57 +41,19 @@
 CHECK:  MemprofProfile:
 CHECK-NEXT:  Summary:
 CHECK-NEXT:Version: 1
-CHECK-NEXT:NumSegments: 9
+CHECK-NEXT:NumSegments: {{[0-9]+}}
 CHECK-NEXT:NumMibInfo: 2
 CHECK-NEXT:NumAllocFunctions: 2
 CHECK-NEXT:NumStackOffsets: 1
 CHECK-NEXT:  Segments:
 CHECK-NEXT:  -
 CHECK-NEXT:BuildId: 
-CHECK-NEXT:Start: 0x20
-CHECK-NEXT:End: 0x29B000
-CHECK-NEXT:Offset: 0x0
+CHECK-NEXT:Start: 0x{{[0-9]+}}
+CHECK-NEXT:End: 0x{{[0-9]+}}
+CHECK-NEXT:Offset: 0x{{[0-9]+}}
 CHECK-NEXT:  -
-CHECK-NEXT:BuildId: 
-CHECK-NEXT:Start: 0x7F5871485000
-CHECK-NEXT:End: 0x7F58715CD000
-CHECK-NEXT:Offset: 0x26000
-CHECK-NEXT:  -
-CHECK-NEXT:BuildId: 
-CHECK-NEXT:Start: 0x7F587162D000
-CHECK-NEXT:End: 0x7F587163F000
-CHECK-NEXT:Offset: 0x3000
-CHECK-NEXT:  -
-CHECK-NEXT:BuildId: 
-CHECK-NEXT:Start: 0x7F5871646000
-CHECK-NEXT:End: 0x7F5871648000
-CHECK-NEXT:Offset: 0x2000
-CHECK-NEXT:  -
-CHECK-NEXT:BuildId: 
-CHECK-NEXT:Start: 0x7F587165A000
-CHECK-NEXT:End: 0x7F58716F4000
-CHECK-NEXT:Offset: 0xF000
-CHECK-NEXT:  -
-CHECK-NEXT:BuildId: 
-CHECK-NEXT:Start: 0x7F5871791000
-CHECK-NEXT:End: 0x7F5871795000
-CHECK-NEXT:Offset: 0x3000
-CHECK-NEXT:  -
-CHECK-NEXT:BuildId: 
-CHECK-NEXT:Start: 0x7F58717A
-CHECK-NEXT:End: 0x7F58717AF000
-CHECK-NEXT:Offset: 0x7000
-CHECK-NEXT:  -
-CHECK-NEXT:BuildId: 
-CHECK-NEXT:Start: 0x7F58717D6000
-CHECK-NEXT:End: 0x7F58717FA000
-CHECK-NEXT:Offset: 0x1000
-CHECK-NEXT:  -
-CHECK-NEXT:BuildId: 
-CHECK-NEXT:Start: 0x7FFFC77BD000
-CHECK-NEXT:End: 0x7FFFC77BF000
-CHECK-NEXT:Offset: 0x0
-CHECK-NEXT:  Records:
+
+CHECK:  Records:
 CHECK-NEXT:  -
 CHECK-NEXT:FunctionGUID: 15505678318020221912
 CHECK-NEXT:AllocSites:
@@ -129,18 +91,24 @@
 CHECK-NEXT:TotalSize: 1
 CHECK-NEXT:MinSize: 1
 CHECK-NEXT:MaxSize: 1
-CHECK-NEXT:AllocTimestamp: 894
-CHECK-NEXT:DeallocTimestamp: 894
+CHECK-NEXT:AllocTimestamp: {{[0-9]+}}
+CHECK-NEXT:DeallocTimestamp: {{[0-9]+}}
 CHECK-NEXT:TotalLifetime: 0
 CHECK-NEXT:MinLifetime: 0
 CHECK-NEXT:MaxLifetime: 0
-CHECK-NEXT:AllocCpuId: 23
-CHECK-NEXT:DeallocCpuId: 23
+CHECK-NEXT:AllocCpuId: {{[0-9]+}}
+CHECK-NEXT:DeallocCpuId: {{[0-9]+}}
 CHECK-NEXT:NumMigratedCpu: 0
 CHECK-NEXT:NumLifetimeOverlaps: 0
 CHECK-NEXT:

[PATCH] D129570: [clang-tidy] Add new clang-tidy check to find implicit conversions from enum to integer.

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.cpp:45
+  } else {
+Note << FixItHint::CreateInsertion(MatchedExpr->getBeginLoc(), "(int)");
+  }

carlosgalvezp wrote:
> I don't think we can assume the type of the enum is `int`, see for example:
> 
> ```
> enum Foo : unsigned int
> {
> a,
> b
> };
> 
> 
> void f(unsigned int);
> 
> int main()
> {
> f(a);
> }
> 
> ```
> 
> Even if there is no explicit underlying type, isn't the underlying type 
> implementation-defined?
Since its an explicit cast then we should probably use the type that the 
function accepts(since thats what it will be eventually converted to)  rather 
than the type of the underling enum type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129570

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

[PATCH] D141338: [WIP][-Wunsafe-buffer-usage] Filter out conflicting fix-its

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 added a comment.

In D141338#4042050 , @NoQ wrote:

> Looks awesome!
>
> I'm worried that the test is going to rapidly become stale as you develop 
> fixits for `DeclStmt` further. It might make sense to write some 
> //unittests// for this class directly, to put into 
> `clang/unittest/Analysis/...`. Then you can create some fixits defined by 
> hand, without invoking UnsafeBufferUsage analysis, and feed them to this 
> class directly and verify the results.
>
> Also let's explain motivation a bit more. Normally compiler warnings don't 
> need such machine because the fixits they come with are very simple and 
> hand-crafted. However, unsafe buffer usage analysis is a large emergent 
> behavior machine that may fix different parts of user code, and different 
> parts of the machine that can produce parts of such fixits aren't necessarily 
> aware of each other. In some cases no progress can be made without making 
> sure these parts of the machine talk to each other. However, every time the 
> machine does emit individual fixits, they're actually correct; conflicts 
> between such fixits are entirely a representation problem (we're unable to 
> present overlapping fixits to the user because this requires us to resolve 
> the merge conflict), not an underlying correctness problem. So @ziqingluo-90 
> is implementing a bailout for the situation when the fixits were perfectly 
> correct in isolation but can't be properly displayed to the user due to merge 
> conflicts between them. This makes it possible for such "merge conflicts" 
> detection to be purely SourceRange-based, so it doesn't need to take the 
> details of the underlying AST into account.

Thanks for adding the motivation here!

I updated the revision by getting rid of things we don't need for now.   
At this point, we just need to tell if there is any conflict in the fix-its 
generated for one variable (including variable declaration fix-its and variable 
usage fix-its).  If there is any,  we do NOT emit any fix-it for that variable.

At some point later, we may want to know the exact conflicting subsets.  That 
will be another patch.


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

https://reviews.llvm.org/D141338

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> Yeah, my thoughts too. Though if we don't compile with -g, does that mean if 
> I use DILocation (or w/e metadata for "inlining occurred") that .debug_info 
> will get emitted, even if the user didn't ask for it? If not, I can probably 
> switch everything to use DILocation.

clang has a "LocTrackingOnly" setting for debug info, which emits DILocation 
info into the IR, but emits a marker into the DICompileUnit to skip emitting 
the .debug_info in the backend.  We currently use it for -Rpass.  We don't do 
this by default, I think to save compile time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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

[PATCH] D141338: [WIP][-Wunsafe-buffer-usage] Filter out conflicting fix-its

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 updated this revision to Diff 488429.

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

https://reviews.llvm.org/D141338

Files:
  clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/unittests/Analysis/CMakeLists.txt
  clang/unittests/Analysis/UnsafeBufferUsageTest.cpp

Index: clang/unittests/Analysis/UnsafeBufferUsageTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/UnsafeBufferUsageTest.cpp
@@ -0,0 +1,60 @@
+#include "gtest/gtest.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
+
+using namespace clang;
+
+namespace {
+// The test fixture.
+class UnsafeBufferUsageTest : public ::testing::Test {
+protected:
+  UnsafeBufferUsageTest()
+  : FileMgr(FileMgrOpts), DiagID(new DiagnosticIDs()),
+Diags(DiagID, new DiagnosticOptions, new IgnoringDiagConsumer()),
+SourceMgr(Diags, FileMgr) {}
+
+  FileSystemOptions FileMgrOpts;
+  FileManager FileMgr;
+  IntrusiveRefCntPtr DiagID;
+  DiagnosticsEngine Diags;
+  SourceManager SourceMgr;
+};
+} // namespace
+
+TEST_F(UnsafeBufferUsageTest, FixItHintsConflict) {
+  const FileEntry *DummyFile = FileMgr.getVirtualFile("", 100, 0);
+  FileID DummyFileID = SourceMgr.getOrCreateFileID(DummyFile, SrcMgr::C_User);
+  SourceLocation StartLoc = SourceMgr.getLocForStartOfFile(DummyFileID);
+
+#define MkDummyHint(Begin, End)\
+  FixItHint::CreateReplacement(SourceRange(StartLoc.getLocWithOffset((Begin)), \
+   StartLoc.getLocWithOffset((End))),  \
+   "dummy")
+
+  FixItHint H1 = MkDummyHint(0, 5);
+  FixItHint H2 = MkDummyHint(6, 10);
+  FixItHint H3 = MkDummyHint(20, 25);
+  llvm::SmallVector Fixes;
+
+  // Test non-overlapping fix-its:
+  Fixes = {H1, H2, H3};
+  EXPECT_FALSE(anyConflict(SourceMgr, Fixes));
+  Fixes = {H3, H2, H1}; // re-order
+  EXPECT_FALSE(anyConflict(SourceMgr, Fixes));
+
+  // Test overlapping fix-its:
+  Fixes = {H1, H2, H3, MkDummyHint(0, 4) /* included in H1 */};
+  EXPECT_TRUE(anyConflict(SourceMgr, Fixes));
+
+  Fixes = {H1, H2, H3, MkDummyHint(10, 15) /* overlaps H2 */};
+  EXPECT_TRUE(anyConflict(SourceMgr, Fixes));
+
+  Fixes = {H1, H2, H3, MkDummyHint(7, 23) /* overlaps H2, H3 */};
+  EXPECT_TRUE(anyConflict(SourceMgr, Fixes));
+
+  Fixes = {H1, H2, H3, MkDummyHint(2, 23) /* overlaps H1, H2, and H3 */};
+  EXPECT_TRUE(anyConflict(SourceMgr, Fixes));
+}
\ No newline at end of file
Index: clang/unittests/Analysis/CMakeLists.txt
===
--- clang/unittests/Analysis/CMakeLists.txt
+++ clang/unittests/Analysis/CMakeLists.txt
@@ -9,6 +9,7 @@
   CloneDetectionTest.cpp
   ExprMutationAnalyzerTest.cpp
   MacroExpansionContextTest.cpp
+  UnsafeBufferUsageTest.cpp
   )
 
 clang_target_link_libraries(ClangAnalysisTests
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -109,7 +109,7 @@
 
 AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher, innerMatcher) {
   const DynTypedMatcher  = static_cast(innerMatcher);
-  
+
   MatchDescendantVisitor Visitor(, Finder, Builder, ASTMatchFinder::BK_All);  
   return Visitor.findMatch(DynTypedNode::create(Node));
 }
@@ -690,6 +690,37 @@
   }
 }
 
+bool clang::anyConflict(const SourceManager ,
+const llvm::SmallVectorImpl ) {
+  // A simple interval overlap detecting algorithm.  Sorts all ranges by their
+  // begin location then find any overlap in one pass.
+  std::vector All; // a copy of `FixIts`
+
+  for (const FixItHint  : FixIts)
+All.push_back();
+  std::sort(All.begin(), All.end(),
+[](const FixItHint *H1, const FixItHint *H2) {
+  return SM.isBeforeInTranslationUnit(H1->RemoveRange.getBegin(),
+  H2->RemoveRange.getBegin());
+});
+
+  const FixItHint *CurrHint = nullptr;
+
+  for (const FixItHint *Hint : All) {
+if (!CurrHint ||
+SM.isBeforeInTranslationUnit(CurrHint->RemoveRange.getEnd(),
+ Hint->RemoveRange.getBegin())) {
+  // Either to initialize `CurrHint` or `CurrHint` does not
+  // overlap with `Hint`:
+  CurrHint = Hint;
+} else
+  // In case `Hint` overlaps the `CurrHint`, we found at least one
+  // conflict:
+  return true;
+  }
+  return false;
+}
+
 void clang::checkUnsafeBufferUsage(const Decl *D,
UnsafeBufferUsageHandler ) {
   assert(D && D->getBody());
@@ -777,9 +808,7 @@
   Fixes = 

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[Felipe de Azevedo Piovezan via Phabricator via cfe-commits]

fdeazeve added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.h:494
+   CGBuilderTy ,
+   const bool UsePointerValue = false);
 

dblaikie wrote:
> fdeazeve wrote:
> > FWIW I used a `const` bool here to match the style already present in this 
> > class
> Examples of the things you were trying to be consistent with? Because the 
> other by-value parameter here isn't const at the top level & const at the top 
> level on function declaration parameters has no meaning, so especially here 
> it should probably be omitted (& probably also in the definition, but it's a 
> somewhat separate issue/has different tradeoffs)
The function on line 475 uses this same style:


```
  llvm::DILocalVariable *
  EmitDeclareOfAutoVariable(const VarDecl *Decl, llvm::Value *AI,
CGBuilderTy ,
const bool UsePointerValue = false);
```

That said, I agree with you and personally would not have added const here, 
I'll remove it and then later we can maybe clean up other functions in this file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141381

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

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 488427.
paulkirth added a comment.

Update clang test for windows file separators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

Files:
  clang/test/Frontend/stack-layout-remark.c
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/test/CodeGen/AArch64/O0-pipeline.ll
  llvm/test/CodeGen/AArch64/O3-pipeline.ll
  llvm/test/CodeGen/AArch64/arm64-opt-remarks-lazy-bfi.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/ARM/stack-frame-layout-remarks.ll
  llvm/test/CodeGen/Generic/llc-start-stop.ll
  llvm/test/CodeGen/PowerPC/O0-pipeline.ll
  llvm/test/CodeGen/PowerPC/O3-pipeline.ll
  llvm/test/CodeGen/RISCV/O0-pipeline.ll
  llvm/test/CodeGen/RISCV/O3-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll
  llvm/test/CodeGen/X86/stack-frame-layout-remarks.ll

Index: llvm/test/CodeGen/X86/stack-frame-layout-remarks.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-frame-layout-remarks.ll
@@ -0,0 +1,315 @@
+; Test remark output for stack-frame-layout
+
+; ensure basic output works
+; RUN: llc -mcpu=corei7 -O1 -pass-remarks-analysis=stack-frame-layout < %s 2>&1 >/dev/null | FileCheck %s
+
+; check additional slots are displayed when stack is not optimized
+; RUN: llc -mcpu=corei7 -O0 -pass-remarks-analysis=stack-frame-layout < %s 2>&1 >/dev/null | FileCheck %s --check-prefix=NO_COLORING
+
+; check more complex cases
+; RUN: llc %s -pass-remarks-analysis=stack-frame-layout -o /dev/null --march=x86 -mcpu=i386 2>&1 | FileCheck %s --check-prefix=BOTH --check-prefix=DEBUG
+
+; check output without debug info
+; RUN: opt %s -passes=strip -S | llc  -pass-remarks-analysis=stack-frame-layout -o /dev/null --march=x86 -mcpu=i386 2>&1 | FileCheck %s --check-prefix=BOTH --check-prefix=STRIPPED
+
+target triple = "x86_64-unknown-linux-gnu"
+
+@.str = private unnamed_addr constant [4 x i8] c"%s\0A\00", align 1
+declare i32 @printf(ptr, ...)
+
+; CHECK: Function: stackSizeWarning
+; CHECK: Offset: [SP-88], Type: Variable, Align: 16, Size: 80
+; CHECK:buffer @ frame-diags.c:30
+; NO_COLORING: Offset: [SP-168], Type: Variable, Align: 16, Size: 80
+; CHECK:buffer2 @ frame-diags.c:33
+define void @stackSizeWarning() {
+entry:
+  %buffer = alloca [80 x i8], align 16
+  %buffer2 = alloca [80 x i8], align 16
+  call void @llvm.dbg.declare(metadata ptr %buffer, metadata !25, metadata !DIExpression()), !dbg !39
+  call void @llvm.dbg.declare(metadata ptr %buffer2, metadata !31, metadata !DIExpression()), !dbg !40
+  ret void
+}
+
+; Function Attrs: nocallback nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+
+; BOTH: Function: cleanup_array
+; BOTH-Next:  Offset: [SP+4], Type: Protector, Align: 16, Size: 4
+; DEBUG: a @ dot.c:13
+; STRIPPED-NOT: a @ dot.c:13
+; BOTH:  Offset: [SP-4], Type: Spill, Align: 8, Size: 4
+define void @cleanup_array(ptr %0) #1 {
+  %2 = alloca ptr, align 8
+  store ptr %0, ptr %2, align 8
+  call void @llvm.dbg.declare(metadata ptr %2, metadata !41, metadata !DIExpression()), !dbg !46
+  ret void
+}
+
+; BOTH: Function: cleanup_result
+; BOTH:  Offset: [SP+4], Type: Protector, Align: 16, Size: 4
+; DEBUG: res @ dot.c:21
+; STRIPPED-NOT: res @ dot.c:21
+; BOTH:  Offset: [SP-4], Type: Spill, Align: 8, Size: 4
+define void @cleanup_result(ptr %0) #1 {
+  %2 = alloca ptr, align 8
+  store ptr %0, ptr %2, align 8
+  call void @llvm.dbg.declare(metadata ptr %2, metadata !47, metadata !DIExpression()), !dbg !51
+  ret void
+}
+
+; BOTH: Function: do_work
+; BOTH:  Offset: [SP+12], Type: Variable, Align: 8, Size: 4
+; DEBUG: out @ dot.c:32
+; STRIPPED-NOT: out @ dot.c:32
+; BOTH:  Offset: [SP+8], Type: Variable, Align: 4, Size: 4
+; BOTH:  Offset: [SP+4], Type: Protector, Align: 16, Size: 4
+; DEBUG: A @ dot.c:32
+; STRIPPED-NOT: A @ dot.c:32
+; BOTH:  Offset: [SP-4], Type: Spill, Align: 8, Size: 4
+; BOTH:  Offset: [SP-12], Type: Variable, Align: 8, Size: 4
+; DEBUG: AB @ dot.c:38
+; STRIPPED-NOT: AB @ dot.c:38
+; BOTH:  Offset: [SP-16], Type: Variable, Align: 4, Size: 4
+; DEBUG: i @ dot.c:55
+; STRIPPED-NOT: i @ dot.c:55
+; BOTH:  Offset: [SP-20], Type: Variable, Align: 8, Size: 4
+; DEBUG: B @ dot.c:32
+; STRIPPED-NOT: B @ dot.c:32
+; BOTH:  Offset: [SP-24], Type: Variable, Align: 4, Size: 4
+; DEBUG: len @ dot.c:37
+; STRIPPED-NOT: len @ dot.c:37
+; BOTH:  Offset: [SP-28], Type: Variable, Align: 4, Size: 4
+; BOTH:  Offset: [SP-32], Type: Variable, Align: 4, Size: 4
+; DEBUG: sum @ dot.c:54
+; STRIPPED-NOT: sum @ dot.c:54
+define i32 @do_work(ptr %0, ptr %1, ptr %2) 

[PATCH] D141056: [SVE][CGBuiltins] Remove need for instcombine from ACLE tests.

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm marked an inline comment as done.
paulwalker-arm added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9092
+  if (BytesPerElt > 1) {
+Value *Scale = ConstantInt::get(Int64Ty, Log2_32(BytesPerElt));
+Ops[2] = Builder.CreateShl(Ops[2], Scale);

david-arm wrote:
> Given this seems a frequent idiom is it worth putting this into a helper 
> routine? i.e. something like
> 
>   Ops[2] = getScaledOffset(Ops[2], BytesPerElt);
> 
> where
> 
>   Value *getScaledOffset(SDValue Offset, unsigned Bytes) {
> Value *Scale = ConstantInt::get(Int64Ty, Log2_32(Bytes));
> return Builder.CreateShl(Offset, Scale);
>   }
Thanks Dave. It turns out IRBuilder has such a helper (well almost) function so 
I've used that instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141056

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

[PATCH] D141056: [SVE][CGBuiltins] Remove need for instcombine from ACLE tests.

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm updated this revision to Diff 488418.
paulwalker-arm added a comment.

Rebase and use simpler IRBuilder interface.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141056

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_acge.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_acgt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_acle.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_aclt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_add.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adda.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_addv.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrh.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrw.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_and.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_andv.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asr.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfdot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfmlalb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfmlalt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfmmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bic.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brka.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkn.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkpa.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkpb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cadd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clasta-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clastb-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cls.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clz.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpeq.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpge.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpgt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmple.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmplt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpne.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpuo.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnt-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntp.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_compact.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create3.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create4.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cvt-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cvt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cvtnt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_div.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_divr.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_eor.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_eorv.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_expa.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_extb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_exth.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_extw.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get2.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get3.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get4.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_index.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_insr-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_insr.c
  

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

[Eli Friedman via Phabricator via cfe-commits]

efriedma added subscribers: nikic, efriedma.
efriedma added reviewers: nikic, jdoerfert.
efriedma added a comment.

From an IR semantics standpoint, I'm not sure the `memory(none)` marking is 
right if the function throws an exception.  LangRef doesn't explicitly exclude 
the possibility, but take the following:

  void f() { throw 1; }

It gets lowered to:

  define dso_local void @_Z1fv() local_unnamed_addr #0 {
  entry:
%exception = tail call ptr @__cxa_allocate_exception(i64 4) #1
store i32 1, ptr %exception, align 16, !tbaa !5
tail call void @__cxa_throw(ptr nonnull %exception, ptr nonnull @_ZTIi, ptr 
null) #2
unreachable
  }

So we have a function that essentially makes a heap allocation, store a value 
to it, then throws it.  And throwing the exception then makes other changes to 
the exception handling state which are visible to the caller.  Given these 
details are exposed in LLVM IR, saying that this is `memory(none)` seems a bit 
dubious.  (I don't have a testcase that breaks off the top of my head, but I 
suspect it's possible to write such a testcase.)  Given that, if you're going 
to drop the nounwind attribute, I suspect it also makes sense to also drop the 
`memory(none)` attribute.

The AddPotentialArgAccess() helper exists to handle a similar sort of 
situation: a function is marked `pure` in C, but the return value is returned 
indirectly, so it's not actually pure from the perspective of LLVM IR.  To fix 
that, we adjust the IR attributes to something more appropriate.

(Adding @nikic and @jdoerfert for additional perspective on the IR attributes.)

-

See also https://github.com/llvm/llvm-project/issues/36098 , which is also 
about a mismatch between the meaning of the C `pure` attribute and the LLVM IR 
`memory(none)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138958

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

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

[Stefan Gränitz via Phabricator via cfe-commits]

sgraenitz added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137944

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

[PATCH] D141550: [CompilerRT] Remove ubsan static runtime on Apple

[Dan Liew via Phabricator via cfe-commits]

delcypher added inline comments.



Comment at: compiler-rt/lib/ubsan/CMakeLists.txt:118
+if (NOT APPLE)
+  add_compiler_rt_runtime(clang_rt.ubsan
+STATIC

usama54321 wrote:
> delcypher wrote:
> > I think you may have accidentally added tabs here when re-indenting.
> I double checked and these are spaces :/
Oh my bad. Maybe the `>>` symbol in phrabricator means indent rather than a 
particular space/tab choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141550

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

[PATCH] D141550: [CompilerRT] Remove ubsan static runtime on Apple

[Usama Hameed via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:219
+def err_drv_unsupported_static_ubsan_darwin : Error<
+  "Static UBSan runtime is not supported on darwin">;
 def err_drv_duplicate_config : Error<

delcypher wrote:
> Nit: Driver messages start with lowercase.
> 
> Also please check if "UBSan" is spelt differently in existing driver 
> messages. It might actually be written more explicitly like "undefined 
> behavior sanitizer".
renamed UBSan to UndefinedBehaviorSanitizer as I see instances of 
AddressSanitizer and ThreadSanitizer in other messages. Did not find an 
equivalent for ubsan


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141550

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

[PATCH] D141175: [test] Split out Annotations from `TestingSupport`

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.

This looks good, i think it also doesn't belong in Testing/Support because it's 
not a peer to anything in Support/.

I think you can use `PARTIAL_SOURCES_INTENDED` to partition directories, but 
it's unusual and a bit of a hassle. Either way seems fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141175

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D141451#4045414 , @efriedma wrote:

> There are two different approaches we use for getting source locations for 
> diagnostics coming out of the backend; either embedding clang SourceLocations 
> into IR, or trying to translate DWARF locations.  The advantage of using 
> clang SourceLocations is that we never lose any fidelity, even in complex 
> cases involving the preprocessor, and the compiler doesn't have to spend time 
> generating complete DWARF info.  The advantage of DWARF, of course, is that 
> it's existing, mature infrastructure.
>
> We use clang SourceLocations for inline asm and for the warning/error 
> attributes, we use DWARF for optimization remarks.

Yep.

> There's probably some argument for using DWARF here.

Yeah, my thoughts too. Though if we don't compiler with `-g`, does that mean if 
I use DILocation (or w/e metadata for "inlining occurred") that .debug_info 
will get emitted, even if the user didn't ask for it?  If not, I can probably 
switch everything to use DILocation.

> 
>
> Am I reading the correctly, that currently the inlining notes don't have 
> source locations?  That's not very friendly (particularly for display in 
> IDEs).

Correct; we pessimistically don't record the srcloc of every call ever. We'd 
need to do that to support line info on the inlining notes. That seems very 
expensive.

Perhaps we only show these notes if debug info is enabled (`-g`) where we can 
then use DiagnosticInfoWithLocationBase? (Do we have precendent for warnings 
that are better with DebugInfo enabled?)

That said, the information provided is A MASSIVE LEAP FORWARD in debugging 
these errors (mostly observed in FORTIFY_SOURCE implementations in the Linux 
kernel) by simply providing the call chain, even without line/col numbers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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

[PATCH] D141172: [ModuleUtils][KCFI] Set patchable-function-prefix for synthesized functions

[Sami Tolvanen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd5e26270660: [ModuleUtils][KCFI] Set 
patchable-function-prefix for synthesized functions (authored by samitolvanen).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141172

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/kcfi.c
  llvm/lib/Transforms/Utils/ModuleUtils.cpp
  llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll


Index: llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
@@ -0,0 +1,15 @@
+;; Test that we set patchable-function-prefix for asan.module_ctor when 
kcfi-offset is defined.
+
+; RUN: opt < %s -passes=asan -S | FileCheck %s
+
+; CHECK: @llvm.global_ctors = {{.*}}{ i32 1, ptr @asan.module_ctor, ptr 
@asan.module_ctor }
+
+; CHECK: define internal void @asan.module_ctor()
+; CHECK-SAME: #[[#ATTR:]]
+; CHECK-SAME: !kcfi_type
+
+; CHECK: attributes #[[#ATTR]] = { {{.*}} "patchable-function-prefix"="3" }
+
+!llvm.module.flags = !{!0, !1}
+!0 = !{i32 4, !"kcfi", i32 1}
+!1 = !{i32 4, !"kcfi-offset", i32 3}
Index: llvm/lib/Transforms/Utils/ModuleUtils.cpp
===
--- llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -161,6 +161,13 @@
   MDNode::get(Ctx, MDB.createConstant(ConstantInt::get(
Type::getInt32Ty(Ctx),
static_cast(xxHash64(MangledType));
+  // If the module was compiled with -fpatchable-function-entry, ensure
+  // we use the same patchable-function-prefix.
+  if (auto *MD = mdconst::extract_or_null(
+  M.getModuleFlag("kcfi-offset"))) {
+if (unsigned Offset = MD->getZExtValue())
+  F.addFnAttr("patchable-function-prefix", std::to_string(Offset));
+  }
 }
 
 FunctionCallee
Index: clang/test/CodeGen/kcfi.c
===
--- clang/test/CodeGen/kcfi.c
+++ clang/test/CodeGen/kcfi.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi 
-o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi 
-x c++ -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi 
-fpatchable-function-entry-offset=3 -o - %s | FileCheck %s 
--check-prefixes=CHECK,OFFSET
 #if !__has_feature(kcfi)
 #error Missing kcfi?
 #endif
@@ -54,5 +55,6 @@
 }
 
 // CHECK-DAG: ![[#]] = !{i32 4, !"kcfi", i32 1}
+// OFFSET-DAG: ![[#]] = !{i32 4, !"kcfi-offset", i32 3}
 // CHECK-DAG: ![[#TYPE]] = !{i32 [[#HASH]]}
 // CHECK-DAG: ![[#TYPE2]] = !{i32 [[#%d,HASH2:]]}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -758,8 +758,14 @@
   CodeGenOpts.SanitizeCfiCanonicalJumpTables);
   }
 
-  if (LangOpts.Sanitize.has(SanitizerKind::KCFI))
+  if (LangOpts.Sanitize.has(SanitizerKind::KCFI)) {
 getModule().addModuleFlag(llvm::Module::Override, "kcfi", 1);
+// KCFI assumes patchable-function-prefix is the same for all indirectly
+// called functions. Store the expected offset for code generation.
+if (CodeGenOpts.PatchableFunctionEntryOffset)
+  getModule().addModuleFlag(llvm::Module::Override, "kcfi-offset",
+CodeGenOpts.PatchableFunctionEntryOffset);
+  }
 
   if (CodeGenOpts.CFProtectionReturn &&
   Target.checkCFProtectionReturnSupported(getDiags())) {


Index: llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
@@ -0,0 +1,15 @@
+;; Test that we set patchable-function-prefix for asan.module_ctor when kcfi-offset is defined.
+
+; RUN: opt < %s -passes=asan -S | FileCheck %s
+
+; CHECK: @llvm.global_ctors = {{.*}}{ i32 1, ptr @asan.module_ctor, ptr @asan.module_ctor }
+
+; CHECK: define internal void @asan.module_ctor()
+; CHECK-SAME: #[[#ATTR:]]
+; CHECK-SAME: !kcfi_type
+
+; CHECK: attributes #[[#ATTR]] = { {{.*}} "patchable-function-prefix"="3" }
+
+!llvm.module.flags = !{!0, !1}
+!0 = !{i32 4, !"kcfi", i32 1}
+!1 = !{i32 4, !"kcfi-offset", i32 3}
Index: llvm/lib/Transforms/Utils/ModuleUtils.cpp
===
--- llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -161,6 +161,13 @@
   MDNode::get(Ctx, MDB.createConstant(ConstantInt::get(
Type::getInt32Ty(Ctx),
static_cast(xxHash64(MangledType));
+  

[clang] fd5e262 - [ModuleUtils][KCFI] Set patchable-function-prefix for synthesized functions

[Sami Tolvanen via cfe-commits]

Author: Sami Tolvanen
Date: 2023-01-11T23:45:49Z
New Revision: fd5e2627066075f3d15ef774ef368e08735a9ac9

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

LOG: [ModuleUtils][KCFI] Set patchable-function-prefix for synthesized functions

When -fpatchable-function-entry is used to emit prefix nops
before functions, KCFI assumes all indirectly called functions
have the same number of prefix nops, because the nops are emitted
between the KCFI type hash and the function entry. However, as
patchable-function-prefix is a function attribute set by Clang,
functions later synthesized by LLVM don't inherit this attribute
and end up not having prefix nops. One of these functions
is asan.module_ctor, which the Linux kernel ends up calling
indirectly when KASAN is enabled.

In order to avoid tripping KCFI, save the expected prefix offset
to a module flag, and use it when we're setting KCFI type for the
relevant synthesized functions.

Link: https://github.com/ClangBuiltLinux/linux/issues/1742

Reviewed By: MaskRay

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

Added: 
llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll

Modified: 
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/kcfi.c
llvm/lib/Transforms/Utils/ModuleUtils.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index c8e27b2f975ca..b85d8926941a5 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -758,8 +758,14 @@ void CodeGenModule::Release() {
   CodeGenOpts.SanitizeCfiCanonicalJumpTables);
   }
 
-  if (LangOpts.Sanitize.has(SanitizerKind::KCFI))
+  if (LangOpts.Sanitize.has(SanitizerKind::KCFI)) {
 getModule().addModuleFlag(llvm::Module::Override, "kcfi", 1);
+// KCFI assumes patchable-function-prefix is the same for all indirectly
+// called functions. Store the expected offset for code generation.
+if (CodeGenOpts.PatchableFunctionEntryOffset)
+  getModule().addModuleFlag(llvm::Module::Override, "kcfi-offset",
+CodeGenOpts.PatchableFunctionEntryOffset);
+  }
 
   if (CodeGenOpts.CFProtectionReturn &&
   Target.checkCFProtectionReturnSupported(getDiags())) {

diff  --git a/clang/test/CodeGen/kcfi.c b/clang/test/CodeGen/kcfi.c
index b0368d59fc744..1adf6df92a654 100644
--- a/clang/test/CodeGen/kcfi.c
+++ b/clang/test/CodeGen/kcfi.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi 
-o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi 
-x c++ -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi 
-fpatchable-function-entry-offset=3 -o - %s | FileCheck %s 
--check-prefixes=CHECK,OFFSET
 #if !__has_feature(kcfi)
 #error Missing kcfi?
 #endif
@@ -54,5 +55,6 @@ int test(void) {
 }
 
 // CHECK-DAG: ![[#]] = !{i32 4, !"kcfi", i32 1}
+// OFFSET-DAG: ![[#]] = !{i32 4, !"kcfi-offset", i32 3}
 // CHECK-DAG: ![[#TYPE]] = !{i32 [[#HASH]]}
 // CHECK-DAG: ![[#TYPE2]] = !{i32 [[#%d,HASH2:]]}

diff  --git a/llvm/lib/Transforms/Utils/ModuleUtils.cpp 
b/llvm/lib/Transforms/Utils/ModuleUtils.cpp
index ee5e65fbaa505..2c2e8db34f278 100644
--- a/llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ b/llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -161,6 +161,13 @@ void llvm::setKCFIType(Module , Function , StringRef 
MangledType) {
   MDNode::get(Ctx, MDB.createConstant(ConstantInt::get(
Type::getInt32Ty(Ctx),
static_cast(xxHash64(MangledType));
+  // If the module was compiled with -fpatchable-function-entry, ensure
+  // we use the same patchable-function-prefix.
+  if (auto *MD = mdconst::extract_or_null(
+  M.getModuleFlag("kcfi-offset"))) {
+if (unsigned Offset = MD->getZExtValue())
+  F.addFnAttr("patchable-function-prefix", std::to_string(Offset));
+  }
 }
 
 FunctionCallee

diff  --git a/llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll 
b/llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
new file mode 100644
index 0..b5d103c073998
--- /dev/null
+++ b/llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
@@ -0,0 +1,15 @@
+;; Test that we set patchable-function-prefix for asan.module_ctor when 
kcfi-offset is defined.
+
+; RUN: opt < %s -passes=asan -S | FileCheck %s
+
+; CHECK: @llvm.global_ctors = {{.*}}{ i32 1, ptr @asan.module_ctor, ptr 
@asan.module_ctor }
+
+; CHECK: define internal void @asan.module_ctor()
+; CHECK-SAME: #[[#ATTR:]]
+; CHECK-SAME: !kcfi_type
+
+; CHECK: attributes #[[#ATTR]] = { {{.*}} "patchable-function-prefix"="3" }
+
+!llvm.module.flags = !{!0, !1}
+!0 = !{i32 4, 

[PATCH] D141550: [CompilerRT] Remove ubsan static runtime on Apple

[Usama Hameed via Phabricator via cfe-commits]

usama54321 marked 3 inline comments as done.
usama54321 added inline comments.



Comment at: compiler-rt/lib/ubsan/CMakeLists.txt:118
+if (NOT APPLE)
+  add_compiler_rt_runtime(clang_rt.ubsan
+STATIC

delcypher wrote:
> I think you may have accidentally added tabs here when re-indenting.
I double checked and these are spaces :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141550

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

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5736
+  C.getArgs().hasArg(options::OPT_fmodule_output) &&
+  C.getArgs().hasArg(options::OPT_o)) {
+SmallString<128> OutputPath;

tahonermann wrote:
> dblaikie wrote:
> > ChuanqiXu wrote:
> > > dblaikie wrote:
> > > > Is there some way we can avoid this (`-fmodule-output  -o ...`) being a 
> > > > special case? It'd be good if we could use a single common 
> > > > implementation regardless of whether `-o` is used (ie: Figure out the 
> > > > output file name (whether it's implicitly determined by clang, in the 
> > > > absence of `-o`, or explicitly provided by the user through `-o`) and 
> > > > then replace the suffix with `pcm`)?
> > > I guess we can't do it or we can't do it easily. Otherwise the .pcm file 
> > > will always lives in the same directory with the input file.
> > > I guess we can't do it or we can't do it easily. Otherwise the .pcm file 
> > > will always lives in the same directory with the input file.
> > 
> > I don't follow/understand. What I mean is, I'd like it, if possible, this 
> > was implemented by something like "find the path for the .o file output, 
> > then replace the extension with .pcm".
> > 
> > I worry atht code like this that recomputes something similar to the object 
> > output path risks getting out of sync with the actual object path.
> That is certainly a valid concern and I agree it would be better if the 
> shared output path is computed exactly once. If that would require 
> significant change, then tests to ensure consistent behavior would be the 
> next best thing. I'm not sure what infrastructure is in place for validation 
> of output file locations though.
Well, I guess the Split DWARF file naming isn't fundamentally better - it looks 
at the OPT_O argument directly too:
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/CommonArgs.cpp#L1250

Perhaps we could at least reuse this same logic - make it a reusable function 
in some way? (for instance, it checks `-c` too, which seems relevant - we 
wouldn't want to name the .pcm after the linked executable name, right?



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

https://reviews.llvm.org/D137058

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

[PATCH] D141550: [CompilerRT] Remove ubsan static runtime on Apple

[Usama Hameed via Phabricator via cfe-commits]

usama54321 updated this revision to Diff 488408.
usama54321 added a comment.

Addressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141550

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/lib/ubsan/CMakeLists.txt
  compiler-rt/test/ubsan/CMakeLists.txt

Index: compiler-rt/test/ubsan/CMakeLists.txt
===
--- compiler-rt/test/ubsan/CMakeLists.txt
+++ compiler-rt/test/ubsan/CMakeLists.txt
@@ -101,7 +101,6 @@
 set(UBSAN_TEST_TARGET_ARCH ${arch})
 get_test_cc_for_arch(${arch} UBSAN_TEST_TARGET_CC UBSAN_TEST_TARGET_CFLAGS)
 set(UBSAN_TEST_TARGET_CFLAGS "${UBSAN_TEST_TARGET_CFLAGS} -lc++abi")
-add_ubsan_testsuites("StandaloneStatic" ubsan ${arch})
   endforeach()
 
   # Device and simulator test suites.
Index: compiler-rt/lib/ubsan/CMakeLists.txt
===
--- compiler-rt/lib/ubsan/CMakeLists.txt
+++ compiler-rt/lib/ubsan/CMakeLists.txt
@@ -114,19 +114,21 @@
   LINK_FLAGS ${WEAK_SYMBOL_LINK_FLAGS}
   PARENT_TARGET ubsan)
 
-add_compiler_rt_runtime(clang_rt.ubsan
-  STATIC
-  OS ${UBSAN_SUPPORTED_OS}
-  ARCHS ${UBSAN_SUPPORTED_ARCH}
-  OBJECT_LIBS RTUbsan
-  RTUbsan_standalone
-  RTSanitizerCommonNoHooks
-  RTSanitizerCommonLibcNoHooks
-  RTSanitizerCommonCoverage
-  RTSanitizerCommonSymbolizerNoHooks
-  RTInterception
-  LINK_FLAGS ${WEAK_SYMBOL_LINK_FLAGS}
-  PARENT_TARGET ubsan)
+if (NOT APPLE)
+  add_compiler_rt_runtime(clang_rt.ubsan
+STATIC
+OS ${UBSAN_SUPPORTED_OS}
+ARCHS ${UBSAN_SUPPORTED_ARCH}
+OBJECT_LIBS RTUbsan
+RTUbsan_standalone
+RTSanitizerCommonNoHooks
+RTSanitizerCommonLibcNoHooks
+RTSanitizerCommonCoverage
+RTSanitizerCommonSymbolizerNoHooks
+RTInterception
+LINK_FLAGS ${WEAK_SYMBOL_LINK_FLAGS}
+PARENT_TARGET ubsan)
+endif()
   endif()
 
 else()
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -423,8 +423,7 @@
 // RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-UBSAN-STATIC-DARWIN %s
-// CHECK-UBSAN-STATIC-DARWIN: "{{.*}}ld{{(.exe)?}}"
-// CHECK-UBSAN-STATIC-DARWIN: "{{.*}}libclang_rt.ubsan_osx.a"
+// CHECK-UBSAN-STATIC-DARWIN: {{.*}}error: static UndefinedBehaviorSanitizer runtime is not supported on darwin
 
 // RUN: %clang -fsanitize=address,undefined -### %s 2>&1 \
 // RUN: --target=i386-unknown-linux -fuse-ld=ld \
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1425,15 +1425,22 @@
   }
 
   const SanitizerArgs  = getSanitizerArgs(Args);
+
+  if (!Sanitize.needsSharedRt() && Sanitize.needsUbsanRt()) {
+getDriver().Diag(diag::err_drv_unsupported_static_ubsan_darwin);
+return;
+  }
+
   if (Sanitize.needsAsanRt())
 AddLinkSanitizerLibArgs(Args, CmdArgs, "asan");
   if (Sanitize.needsLsanRt())
 AddLinkSanitizerLibArgs(Args, CmdArgs, "lsan");
-  if (Sanitize.needsUbsanRt())
+  if (Sanitize.needsUbsanRt()) {
+assert(Sanitize.needsSharedRt() && "Static sanitizer runtimes not supported");
 AddLinkSanitizerLibArgs(Args, CmdArgs,
 Sanitize.requiresMinimalRuntime() ? "ubsan_minimal"
-  : "ubsan",
-Sanitize.needsSharedRt());
+  : "ubsan");
+  }
   if (Sanitize.needsTsanRt())
 AddLinkSanitizerLibArgs(Args, CmdArgs, "tsan");
   if (Sanitize.needsFuzzer() && !Args.hasArg(options::OPT_dynamiclib)) {
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -215,6 +215,8 @@
   "malformed sanitizer coverage allowlist: '%0'">;
 def err_drv_malformed_sanitizer_coverage_ignorelist : Error<
   "malformed sanitizer coverage ignorelist: '%0'">;
+def err_drv_unsupported_static_ubsan_darwin : Error<
+  "static UndefinedBehaviorSanitizer runtime is not supported on darwin">;
 def err_drv_duplicate_config : Error<
   "no more than one option '--config' is allowed">;
 def 

[PATCH] D141551: Require input target triples to have Environment field

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j created this revision.
Herald added a project: All.
lamb-j requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141551

Files:
  clang/lib/Driver/OffloadBundler.cpp


Index: clang/lib/Driver/OffloadBundler.cpp
===
--- clang/lib/Driver/OffloadBundler.cpp
+++ clang/lib/Driver/OffloadBundler.cpp
@@ -107,7 +107,8 @@
 }
 
 bool OffloadTargetInfo::isTripleValid() const {
-  return !Triple.str().empty() && Triple.getArch() != Triple::UnknownArch;
+  return !Triple.str().empty() && Triple.getArch() != Triple::UnknownArch &&
+Triple.hasEnvironment();
 }
 
 bool OffloadTargetInfo::operator==(const OffloadTargetInfo ) const {


Index: clang/lib/Driver/OffloadBundler.cpp
===
--- clang/lib/Driver/OffloadBundler.cpp
+++ clang/lib/Driver/OffloadBundler.cpp
@@ -107,7 +107,8 @@
 }
 
 bool OffloadTargetInfo::isTripleValid() const {
-  return !Triple.str().empty() && Triple.getArch() != Triple::UnknownArch;
+  return !Triple.str().empty() && Triple.getArch() != Triple::UnknownArch &&
+Triple.hasEnvironment();
 }
 
 bool OffloadTargetInfo::operator==(const OffloadTargetInfo ) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139774: [libclang] Add API to set temporary directory location

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

libclang functions like `clang_reparseTranslationUnit_Impl` call 
`clang/lib/Frontend/ASTUnit.cpp:1397` and build the preamble with 
`/*StoreInMemory=*/false`.
If `StoreInMemory` is configurable (true), then you can avoid the temporary 
file `preamble-*.pch`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774

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

[PATCH] D141092: Optionally install clang-tblgen to aid cross-compiling

[James Le Cuirot via Phabricator via cfe-commits]

chewi added a comment.

James Le Cuirot 

I'm new to this process, so thanks for bearing with me.


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

https://reviews.llvm.org/D141092

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

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

[Jan Korous via Phabricator via cfe-commits]

jkorous added inline comments.



Comment at: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h:46
+/// The text indicating that the user needs to provide input there:
+constexpr static const char *const UserFillPlaceHolder = "...";
 } // end namespace clang

jkorous wrote:
> Should we rather pick something that is syntactically incorrect in C++ in 
> order to prevent accidental silent corruption of the sources?
> FWIW Xcode uses `<#placeholder#>` syntax.
Correction - it is not Xcode, it is clang itself.
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/CodeCompleteConsumer.cpp#L323


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

https://reviews.llvm.org/D139737

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

[PATCH] D141280: [clang][wip] Build UsingType for elaborated type specifiers.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 488403.
hokein marked 2 inline comments as done.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141280

Files:
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/drs/dr2xx.cpp
  clang/test/CXX/drs/dr4xx.cpp
  clang/test/SemaCXX/using-decl-1.cpp

Index: clang/test/SemaCXX/using-decl-1.cpp
===
--- clang/test/SemaCXX/using-decl-1.cpp
+++ clang/test/SemaCXX/using-decl-1.cpp
@@ -177,6 +177,10 @@
 
 struct S {
   friend struct HiddenTag1; // expected-error {{tag type that does not match previous}}
+#if __cplusplus < 201103L
+// expected-warning@-2 {{befriending enumeration type 'struct HiddenTag1' is a C++11 extension}}
+#endif
+
   friend struct HiddenTag2; // expected-note {{conflicting declaration}}
   friend void HiddenFn1(); // expected-error {{cannot befriend target of using declaration}}
   friend void HiddenFn2(); // expected-note {{conflicting declaration}}
Index: clang/test/CXX/drs/dr4xx.cpp
===
--- clang/test/CXX/drs/dr4xx.cpp
+++ clang/test/CXX/drs/dr4xx.cpp
@@ -301,9 +301,8 @@
 struct F;
 struct H;
   }
-  // FIXME: This is ill-formed.
   using N::D;
-  struct dr417::D {}; // expected-warning {{extra qualification}}
+  struct dr417::D {}; // expected-error {{forward declaration of struct cannot}} expected-warning {{extra qualification}}
   using namespace N;
   struct dr417::E {}; // expected-warning {{extra qualification}} expected-error {{no struct named 'E'}}
   struct N::F {};
@@ -311,7 +310,7 @@
   using N::H;
   namespace M {
 struct dr417::G {}; // expected-error {{namespace 'M' does not enclose}}
-struct dr417::H {}; // expected-error {{namespace 'M' does not enclose}}
+struct dr417::H {}; // expected-error {{namespace 'M' does not enclose}} expected-error {{forward declaration of struct cannot have}}
   }
 }
 
Index: clang/test/CXX/drs/dr2xx.cpp
===
--- clang/test/CXX/drs/dr2xx.cpp
+++ clang/test/CXX/drs/dr2xx.cpp
@@ -992,7 +992,7 @@
   }
   struct B::V {}; // expected-error {{no struct named 'V'}}
   struct B::W {};
-  struct B::X {}; // FIXME: ill-formed
+  struct B::X {}; // expected-error {{forward declaration of struct cannot have}}
   enum B::Y e; // ok per dr417
   class B::Z z; // ok per dr417
 
@@ -1009,7 +1009,7 @@
   };
   struct D::V {}; // expected-error {{no struct named 'V'}}
   struct D::W {};
-  struct D::X {}; // FIXME: ill-formed
+  struct D::X {}; // expected-error {{forward declaration of struct cannot have}}
   enum D::Y e2; // ok per dr417
   class D::Z z2; // ok per dr417
 }
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1587,6 +1587,9 @@
 
 // TypeQuals handled by caller.
 Result = Context.getTypeDeclType(D);
+if (const auto *Using =
+dyn_cast_or_null(DS.getRepAsFoundDecl()))
+  Result = Context.getUsingType(Using, Result);
 
 // In both C and C++, make an ElaboratedType.
 ElaboratedTypeKeyword Keyword
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -10180,13 +10180,15 @@
 
   bool Owned = false;
   bool IsDependent = false;
+  UsingShadowDecl* FoundUsing = nullptr;
   Decl *TagD = ActOnTag(S, TagSpec, Sema::TUK_Reference,
 KWLoc, SS, Name, NameLoc, Attr, AS_none,
 /*ModulePrivateLoc=*/SourceLocation(),
 MultiTemplateParamsArg(), Owned, IsDependent,
 SourceLocation(), false, TypeResult(),
 /*IsTypeSpecifier*/false,
-/*IsTemplateParamOrArg*/false);
+/*IsTemplateParamOrArg*/false,
+FoundUsing);
   assert(!IsDependent && "explicit instantiation of dependent name not yet handled");
 
   if (!TagD)
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -16964,6 +16964,7 @@
 if (SS.isEmpty()) {
   bool Owned = false;
   bool IsDependent = false;
+  UsingShadowDecl* FoundUsing = nullptr;
   return ActOnTag(S, TagSpec, TUK_Friend, TagLoc, SS, Name, NameLoc,
   

[PATCH] D141437: [HIP] Use .hipi as preprocessor output extension

[Yaxun Liu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Closed by commit rGe8f41fdb5c6f: [HIP] Use .hipi as preprocessor output 
extension (authored by yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D141437?vs=487997=488402#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141437

Files:
  clang/include/clang/Driver/Types.def
  clang/lib/Driver/OffloadBundler.cpp
  clang/lib/Driver/Types.cpp
  clang/test/Driver/hip-rdc-device-only.hip
  clang/test/Driver/hip-save-temps.hip
  clang/test/Driver/hip-unbundle-preproc.hip
  clang/test/Driver/hip-unbundle-preproc.hipi
  clang/test/Driver/lit.local.cfg
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -93,22 +93,23 @@
 TargetNames("targets", cl::CommaSeparated,
 cl::desc("[-,...]"),
 cl::cat(ClangOffloadBundlerCategory));
-  cl::opt
-FilesType("type", cl::Required,
-  cl::desc("Type of the files to be bundled/unbundled.\n"
-   "Current supported types are:\n"
-   "  i   - cpp-output\n"
-   "  ii  - c++-cpp-output\n"
-   "  cui - cuda/hip-output\n"
-   "  d   - dependency\n"
-   "  ll  - llvm\n"
-   "  bc  - llvm-bc\n"
-   "  s   - assembler\n"
-   "  o   - object\n"
-   "  a   - archive of objects\n"
-   "  gch - precompiled-header\n"
-   "  ast - clang AST file"),
-  cl::cat(ClangOffloadBundlerCategory));
+  cl::opt FilesType(
+  "type", cl::Required,
+  cl::desc("Type of the files to be bundled/unbundled.\n"
+   "Current supported types are:\n"
+   "  i- cpp-output\n"
+   "  ii   - c++-cpp-output\n"
+   "  cui  - cuda-cpp-output\n"
+   "  hipi - hip-cpp-output\n"
+   "  d- dependency\n"
+   "  ll   - llvm\n"
+   "  bc   - llvm-bc\n"
+   "  s- assembler\n"
+   "  o- object\n"
+   "  a- archive of objects\n"
+   "  gch  - precompiled-header\n"
+   "  ast  - clang AST file"),
+  cl::cat(ClangOffloadBundlerCategory));
   cl::opt
 Unbundle("unbundle",
  cl::desc("Unbundle bundled file into several output files.\n"),
Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,7 +1,7 @@
 from lit.llvm import llvm_config
 
 config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
-   '.cu', '.rs', '.cl', '.clcpp', '.hip', '.hlsl']
+   '.cu', '.rs', '.cl', '.clcpp', '.hip', '.hipi', '.hlsl']
 config.substitutions = list(config.substitutions)
 config.substitutions.insert(0,
 ('%clang_cc1',
Index: clang/test/Driver/hip-unbundle-preproc.hipi
===
--- clang/test/Driver/hip-unbundle-preproc.hipi
+++ clang/test/Driver/hip-unbundle-preproc.hipi
@@ -4,7 +4,11 @@
 // RUN:   --offload-arch=gfx803 -nogpulib \
 // RUN:   -x hip-cpp-output %s 2>&1 | FileCheck %s
 
-// CHECK: {{".*clang-offload-bundler.*"}} {{.*}}"-output=[[HOST_PP:.*cui]]" "-output=[[DEV_PP:.*cui]]" "-unbundle"
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx803 -nogpulib \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: {{".*clang-offload-bundler.*"}} {{.*}}"-output=[[HOST_PP:.*hipi]]" "-output=[[DEV_PP:.*hipi]]" "-unbundle"
 // CHECK: {{".*clang.*"}} "-cc1" {{.*}}"-target-cpu" "gfx803" {{.*}}"-o" "[[DEV_O:.*o]]" {{.*}}"[[DEV_PP]]"
 // CHECK: {{".*lld.*"}} {{.*}}"-o" "[[DEV_ISA:.*]]" "[[DEV_O]]"
 // CHECK: {{".*clang-offload-bundler.*"}} {{.*}}"-input={{.*}}" "-input=[[DEV_ISA]]" "-output=[[FATBIN:.*]]"
@@ -13,11 +17,11 @@
 
 // RUN: %clang -### --target=x86_64-unknown-linux-gnu \
 // RUN:   --offload-arch=gfx803 -nogpulib -fgpu-rdc \
-// RUN:   -x hip-cpp-output %s 2>&1 | FileCheck -check-prefix=RDC %s
+// RUN:   %s 2>&1 | FileCheck -check-prefix=RDC %s
 
-// RDC: {{".*clang-offload-bundler.*"}} {{.*}}"-output=[[HOST_PP:.*cui]]" "-output=[[DEV_PP:.*cui]]" "-unbundle"
+// RDC: {{".*clang-offload-bundler.*"}} {{.*}}"-output=[[HOST_PP:.*hipi]]" "-output=[[DEV_PP:.*hipi]]" "-unbundle"
 // RDC: {{".*clang.*"}} 

[clang] e8f41fd - [HIP] Use .hipi as preprocessor output extension

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2023-01-11T18:39:58-05:00
New Revision: e8f41fdb5c6fd7903f65216d70c64eb52c898b51

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

LOG: [HIP] Use .hipi as preprocessor output extension

so that clang can recognize it and handle it automatically
without -x hip-cpp-output.

Reviewed by: Artem Belevich

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

Added: 
clang/test/Driver/hip-unbundle-preproc.hipi

Modified: 
clang/include/clang/Driver/Types.def
clang/lib/Driver/OffloadBundler.cpp
clang/lib/Driver/Types.cpp
clang/test/Driver/hip-rdc-device-only.hip
clang/test/Driver/hip-save-temps.hip
clang/test/Driver/lit.local.cfg
clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Removed: 
clang/test/Driver/hip-unbundle-preproc.hip



diff  --git a/clang/include/clang/Driver/Types.def 
b/clang/include/clang/Driver/Types.def
index d00d520d7514..2960f0ba5e9b 100644
--- a/clang/include/clang/Driver/Types.def
+++ b/clang/include/clang/Driver/Types.def
@@ -42,9 +42,9 @@ TYPE("clcpp",CLCXX,PP_CXX,
  "clcpp",  phases
 TYPE("cuda-cpp-output",  PP_CUDA,  INVALID, "cui",
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("cuda", CUDA, PP_CUDA, "cu", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
 TYPE("cuda", CUDA_DEVICE,  PP_CUDA, "cu", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
-TYPE("hip-cpp-output",   PP_HIP,   INVALID, "cui",
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("hip",  HIP,  PP_HIP,  "cu", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
-TYPE("hip",  HIP_DEVICE,   PP_HIP,  "cu", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
+TYPE("hip-cpp-output",   PP_HIP,   INVALID, "hipi",   
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
+TYPE("hip",  HIP,  PP_HIP,  "hip",
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
+TYPE("hip",  HIP_DEVICE,   PP_HIP,  "hip",
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
 TYPE("objective-c-cpp-output",   PP_ObjC,  INVALID, "mi", 
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("objc-cpp-output",  PP_ObjC_Alias, INVALID,"mi", 
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("objective-c",  ObjC, PP_ObjC, "m",  
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)

diff  --git a/clang/lib/Driver/OffloadBundler.cpp 
b/clang/lib/Driver/OffloadBundler.cpp
index 7cffe4008b8e..d304ea5e9281 100644
--- a/clang/lib/Driver/OffloadBundler.cpp
+++ b/clang/lib/Driver/OffloadBundler.cpp
@@ -847,6 +847,8 @@ CreateFileHandler(MemoryBuffer ,
 return std::make_unique(/*Comment=*/"//");
   if (FilesType == "cui")
 return std::make_unique(/*Comment=*/"//");
+  if (FilesType == "hipi")
+return std::make_unique(/*Comment=*/"//");
   // TODO: `.d` should be eventually removed once `-M` and its variants are
   // handled properly in offload compilation.
   if (FilesType == "d")

diff  --git a/clang/lib/Driver/Types.cpp b/clang/lib/Driver/Types.cpp
index ffbca82c98c3..a890cc58ee42 100644
--- a/clang/lib/Driver/Types.cpp
+++ b/clang/lib/Driver/Types.cpp
@@ -294,68 +294,69 @@ bool types::isSrcFile(ID Id) {
 
 types::ID types::lookupTypeForExtension(llvm::StringRef Ext) {
   return llvm::StringSwitch(Ext)
-   .Case("c", TY_C)
-   .Case("C", TY_CXX)
-   .Case("F", TY_Fortran)
-   .Case("f", TY_PP_Fortran)
-   .Case("h", TY_CHeader)
-   .Case("H", TY_CXXHeader)
-   .Case("i", TY_PP_C)
-   .Case("m", TY_ObjC)
-   .Case("M", TY_ObjCXX)
-   .Case("o", TY_Object)
-   .Case("S", TY_Asm)
-   .Case("s", TY_PP_Asm)
-   .Case("bc", TY_LLVM_BC)
-   .Case("cc", TY_CXX)
-   .Case("CC", TY_CXX)
-   .Case("cl", TY_CL)
-   .Case("clcpp", TY_CLCXX)
-   .Case("cp", TY_CXX)
-   .Case("cu", TY_CUDA)
-   .Case("hh", TY_CXXHeader)
-   .Case("ii", TY_PP_CXX)
-   .Case("ll", TY_LLVM_IR)
-   .Case("mi", TY_PP_ObjC)
-   .Case("mm", TY_ObjCXX)
-   .Case("rs", 

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D141381#4045215 , @rjmccall wrote:

> That's about what I would expect.  One or two extra instructions per argument 
> that are trivially removed in debug builds.  Very small overall impact 
> because there just aren't very many of these arguments.
>
> I don't really see a need to post about this on Discourse, but it might be 
> worth a release note.

Fair enough - if you're cool with it. Yeah, release note wouldn't hurt.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4826
+  unsigned ArgNo, CGBuilderTy ,
+  const bool UsePointerValue) {
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());

We don't usually use top-level const on parameters/locals like this, for 
consistency (eg: with `ArgNo`) please remove it. Otherwise it can lead to 
confusion that maybe there was a `&` intended, etc.



Comment at: clang/lib/CodeGen/CGDebugInfo.h:494
+   CGBuilderTy ,
+   const bool UsePointerValue = false);
 

fdeazeve wrote:
> FWIW I used a `const` bool here to match the style already present in this 
> class
Examples of the things you were trying to be consistent with? Because the other 
by-value parameter here isn't const at the top level & const at the top level 
on function declaration parameters has no meaning, so especially here it should 
probably be omitted (& probably also in the definition, but it's a somewhat 
separate issue/has different tradeoffs)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141381

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

[PATCH] D141550: [CompilerRT] Remove ubsan static runtime on Apple

[Dan Liew via Phabricator via cfe-commits]

delcypher added a comment.

Overall approach LGTM. I just have some very minor nits.




Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:219
+def err_drv_unsupported_static_ubsan_darwin : Error<
+  "Static UBSan runtime is not supported on darwin">;
 def err_drv_duplicate_config : Error<

Nit: Driver messages start with lowercase.

Also please check if "UBSan" is spelt differently in existing driver messages. 
It might actually be written more explicitly like "undefined behavior 
sanitizer".



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:1441
 Sanitize.requiresMinimalRuntime() ? "ubsan_minimal"
-  : "ubsan",
-Sanitize.needsSharedRt());
+  : "ubsan");
   if (Sanitize.needsTsanRt())

Maybe add an assert? As the code is constructed right now it should never fail 
but in the future someone could change the code and break the assumption.

```lang=c++
  if (Sanitize.needsUbsanRt()) {
assert(Sanitize.needsSharedRt() && "Static sanitizer runtimes not 
supported");
AddLinkSanitizerLibArgs(Args, CmdArgs,
Sanitize.requiresMinimalRuntime() ? "ubsan_minimal"
  : "ubsan");
}

```



Comment at: compiler-rt/lib/ubsan/CMakeLists.txt:118
+if (NOT APPLE)
+  add_compiler_rt_runtime(clang_rt.ubsan
+STATIC

I think you may have accidentally added tabs here when re-indenting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141550

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

There are two different approaches we use for getting source locations for 
diagnostics coming out of the backend; either embedding clang SourceLocations 
into IR, or trying to translate DWARF locations.  The advantage of using clang 
SourceLocations is that we never lose any fidelity, even in complex cases 
involving the preprocessor, and the compiler doesn't have to spend time 
generating complete DWARF info.  The advantage of DWARF, of course, is that 
it's existing, mature infrastructure.

We use clang SourceLocations for inline asm and for the warning/error 
attributes, we use DWARF for optimization remarks.

There's probably some argument for using DWARF here.



Am I reading the correctly, that currently the inlining notes don't have source 
locations?  That's not very friendly (particularly for display in IDEs).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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

[PATCH] D141550: [CompilerRT] Remove ubsan static runtime on Apple

[Usama Hameed via Phabricator via cfe-commits]

usama54321 created this revision.
Herald added a subscriber: Enna1.
Herald added a project: All.
usama54321 requested review of this revision.
Herald added subscribers: Sanitizers, cfe-commits, MaskRay.
Herald added projects: clang, Sanitizers.

This patch removes the static ubsan runtime on Apple devices. The motivation
is to reduce the toolchain size.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141550

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/lib/ubsan/CMakeLists.txt
  compiler-rt/test/ubsan/CMakeLists.txt

Index: compiler-rt/test/ubsan/CMakeLists.txt
===
--- compiler-rt/test/ubsan/CMakeLists.txt
+++ compiler-rt/test/ubsan/CMakeLists.txt
@@ -101,7 +101,6 @@
 set(UBSAN_TEST_TARGET_ARCH ${arch})
 get_test_cc_for_arch(${arch} UBSAN_TEST_TARGET_CC UBSAN_TEST_TARGET_CFLAGS)
 set(UBSAN_TEST_TARGET_CFLAGS "${UBSAN_TEST_TARGET_CFLAGS} -lc++abi")
-add_ubsan_testsuites("StandaloneStatic" ubsan ${arch})
   endforeach()
 
   # Device and simulator test suites.
Index: compiler-rt/lib/ubsan/CMakeLists.txt
===
--- compiler-rt/lib/ubsan/CMakeLists.txt
+++ compiler-rt/lib/ubsan/CMakeLists.txt
@@ -114,19 +114,21 @@
   LINK_FLAGS ${WEAK_SYMBOL_LINK_FLAGS}
   PARENT_TARGET ubsan)
 
-add_compiler_rt_runtime(clang_rt.ubsan
-  STATIC
-  OS ${UBSAN_SUPPORTED_OS}
-  ARCHS ${UBSAN_SUPPORTED_ARCH}
-  OBJECT_LIBS RTUbsan
-  RTUbsan_standalone
-  RTSanitizerCommonNoHooks
-  RTSanitizerCommonLibcNoHooks
-  RTSanitizerCommonCoverage
-  RTSanitizerCommonSymbolizerNoHooks
-  RTInterception
-  LINK_FLAGS ${WEAK_SYMBOL_LINK_FLAGS}
-  PARENT_TARGET ubsan)
+if (NOT APPLE)
+  add_compiler_rt_runtime(clang_rt.ubsan
+STATIC
+OS ${UBSAN_SUPPORTED_OS}
+ARCHS ${UBSAN_SUPPORTED_ARCH}
+OBJECT_LIBS RTUbsan
+RTUbsan_standalone
+RTSanitizerCommonNoHooks
+RTSanitizerCommonLibcNoHooks
+RTSanitizerCommonCoverage
+RTSanitizerCommonSymbolizerNoHooks
+RTInterception
+LINK_FLAGS ${WEAK_SYMBOL_LINK_FLAGS}
+PARENT_TARGET ubsan)
+endif()
   endif()
 
 else()
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -423,8 +423,7 @@
 // RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-UBSAN-STATIC-DARWIN %s
-// CHECK-UBSAN-STATIC-DARWIN: "{{.*}}ld{{(.exe)?}}"
-// CHECK-UBSAN-STATIC-DARWIN: "{{.*}}libclang_rt.ubsan_osx.a"
+// CHECK-UBSAN-STATIC-DARWIN: {{.*}}error: Static UBSan runtime is not supported on darwin
 
 // RUN: %clang -fsanitize=address,undefined -### %s 2>&1 \
 // RUN: --target=i386-unknown-linux -fuse-ld=ld \
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1425,6 +1425,12 @@
   }
 
   const SanitizerArgs  = getSanitizerArgs(Args);
+
+  if (!Sanitize.needsSharedRt() && Sanitize.needsUbsanRt()) {
+getDriver().Diag(diag::err_drv_unsupported_static_ubsan_darwin);
+return;
+  }
+
   if (Sanitize.needsAsanRt())
 AddLinkSanitizerLibArgs(Args, CmdArgs, "asan");
   if (Sanitize.needsLsanRt())
@@ -1432,8 +1438,7 @@
   if (Sanitize.needsUbsanRt())
 AddLinkSanitizerLibArgs(Args, CmdArgs,
 Sanitize.requiresMinimalRuntime() ? "ubsan_minimal"
-  : "ubsan",
-Sanitize.needsSharedRt());
+  : "ubsan");
   if (Sanitize.needsTsanRt())
 AddLinkSanitizerLibArgs(Args, CmdArgs, "tsan");
   if (Sanitize.needsFuzzer() && !Args.hasArg(options::OPT_dynamiclib)) {
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -215,6 +215,8 @@
   "malformed sanitizer coverage allowlist: '%0'">;
 def err_drv_malformed_sanitizer_coverage_ignorelist : Error<
   "malformed sanitizer coverage ignorelist: '%0'">;
+def err_drv_unsupported_static_ubsan_darwin : Error<
+  "Static UBSan runtime is not supported on darwin">;
 def err_drv_duplicate_config : Error<
   "no more than one option '--config' is allowed">;
 def err_drv_cannot_open_config_file : 

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D137517#4045337 , @jrtc27 wrote:

> In D137517#4045315 , @fpetrogalli 
> wrote:
>
>> In D137517#4045299 , @jrtc27 wrote:
>>
>>> In D137517#4042875 , @fpetrogalli 
>>> wrote:
>>>
 In D137517#4042758 , 
 @fpetrogalli wrote:

> After submitting this, I had to revert it 
> 
>  because of failures like 
> https://lab.llvm.org/buildbot/#/builders/225/builds/12367/steps/5/logs/stdio

 I have resubmitted with what I hope is the right fix (I could not 
 reproduce any of the failures I was seeing in buildbot, on my machine the 
 build is fine).

 The new commit is at 
 https://github.com/llvm/llvm-project/commit/ac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf
  - in the commit message I have explained what I have changed WRT this 
 original patch. I have added  the
 tablegen target `RISCVTargetParserTableGen` in the `DEPENDS` list of 
 `clangDriver` and `clangBasic`. This makes sure that the `.*inc` file with 
 theist o the CPU is available even if `LLVMTargetParser` has not been 
 built yet.
>>>
>>> But you didn't use the proper Differential Revision tag, so the diff here 
>>> didn't get updated to reflect the amended version committed :(
>>
>> What should I have done? Add  the `Differential Revision: 
>> https://reviews.llvm.org/D137517` as the last line of the commit with the 
>> rework? I wasn't aware of this for reworks.
>
> Yes, it should have the same trailer as the original commit, otherwise it 
> won't be correctly tracked by Phabricator. A "rework" isn't special, it's 
> revert, reopen the revision, update the revision and land the revision again. 
> If re-review isn't needed then you can skip some of the middle, but that's 
> it. Though in this case I do think re-review was warranted, the new clang 
> dependency seems a bit dubious and hints at the design not being quite right.

My bad -  I assumed that adding such tablegenning dependency in clang was a 
minor thing. I'll see if I can remove it. Any suggestions on how this could be 
done without duplicating the information in RISCV.td?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

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

[PATCH] D139774: [libclang] Add API to set temporary directory location

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D139774#4044258 , @aaron.ballman 
wrote:

> In D139774#4043886 , @vedgy wrote:
>
>> In D139774#4041308 , 
>> @aaron.ballman wrote:
>>
>>> Is your concern essentially that someone will add a new use to put files in 
>>> a temp directory and not have access to this information from ASTUnit? Or 
>>> do you have other concerns in mind?
>>>
>>> We should certainly investigate whether there are other uses of temp files 
>>> from libclang as part of these changes. It's possible we'll find a 
>>> situation that makes my suggestion untenable because we don't have easy 
>>> access to the information we need.
>>
>> The temporary directory could be used, now or in future, by some support 
>> code, which is used indirectly by libclang. I found the following uses 
>> potentially relevant to libclang:
>>
>> - `Driver::GetTemporaryDirectory(StringRef Prefix)` calls 
>> `llvm::sys::fs::createUniqueDirectory(Prefix, Path);`
>> - `StandardInstrumentations` => `DotCfgChangeReporter` calls 
>> `sys::fs::createUniquePath("cfgdot-%%.dot", SV, true);`
>> - There are plenty of `createTemporaryFile()` uses in llvm and clang. Some 
>> of them are likely used by libclang.
>>
>> I don't know how to efficiently check whether or not each of these indirect 
>> `system_temp_directory()` uses is in turn used by libclang. Even if they 
>> aren't know, they might be later, when libclang API grows.
>
> Oof, that is more exposure than I was thinking we had...
>
>> On a different note, do you think overriding temporary directory path is 
>> useful only to libclang and not likely to be useful to any other LLVM API 
>> users?
>
> I don't think there are any in-tree needs for that functionality, and the 
> solution is fragile enough that I'd like to avoid it if possible (for 
> example, it also makes the API much harder to use across threads because now 
> it's accessing global state). That said, I think the libclang use case you 
> have is compelling and worth solving in-tree if we can (so others don't have 
> to find similar workarounds).
>
 Another issue is with the `FileSystemOptions` part: this class is widely 
 used in Clang. So adding a member to it could adversely affect performance 
 of unrelated code.
>>>
>>> It's good to keep an eye on that, but I'm not too worried about the 
>>> overhead from it (I don't see uses in AST nodes). We can keep an eye on 
>>> https://llvm-compile-time-tracker.com/ to see if there is a surprising 
>>> compile time increase though.
>>
>> [in case we pursue this design approach, which I currently don't feel is 
>> right] Why not add a temporary path data member into `class ASTUnit` under 
>> the `FileSystemOptions FileSystemOpts` member, not inside it? So that other 
>> code is not burdened with unused struct member, and to be able to use a more 
>> suitable type, like `SmallString` for the temporary path buffer.
>
> That's certainly an option, but the design of that would be a bit strange in 
> that we have some file system options in one place and other file system 
> options in another place. I think ultimately, we're going to want them all to 
> live on `FileSystemOptions`. That said, I'm going to rope in some more folks 
> for a wider selection of opinions on how/if to proceed. CC @dblaikie @MaskRay 
> @d0k

Yeah, I think, at a quick glance, I'd lean towards FilesystemOptions too - it's 
mostly used as a member of FileManager, which already has a lot of 
members/fairly large footprint, so I wouldn't worry too much about this having 
a huge impact on the total memory usage/perf/etc.

I'm not sure how I feel about the general premise (though I don't have enough 
context/ownership to want to veto any direction/progress here, just thoughts in 
case they resonate):

1. Should clang be doing something better with these temp files anyway, no 
matter the directory they go in? (setting them for cleanup at process exit or 
the like - I think we have such a filesystem abstraction, maybe that only works 
on Windows? I'm not sure)
2. If there isn't a better general way to avoid creating temp trash that's a 
problem, I'd have thought that KDevelop/other tools would have issues with 
other temp files too, and want a more general solution (I'd have thought 
changing the process's temp directory, and changing it back for user process 
launches, would be sufficient - so it can cleanup after itself for all files, 
not just these particular clang-created ones)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774

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

[PATCH] D141547: Fix format for `case` in .proto files

[Eric Christopher via Phabricator via cfe-commits]

echristo accepted this revision.
echristo 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/D141547/new/

https://reviews.llvm.org/D141547

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

[PATCH] D141545: [OpenMP] Implement `omp_get_mapped_ptr`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: openmp/libomptarget/src/api.cpp:355
+  bool IsHostPtr = false;
+  auto  = *PM->Devices[DeviceNum];
+  TargetPointerResultTy TPR =

jdoerfert wrote:
> And this needs to be (shared) locked too. 
> 
> @carlo.bertolli will fix these up consistently (see also above).
Yeah, they will be needed fixed consistently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141545

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

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D137517#4045315 , @fpetrogalli 
wrote:

> In D137517#4045299 , @jrtc27 wrote:
>
>> In D137517#4042875 , @fpetrogalli 
>> wrote:
>>
>>> In D137517#4042758 , @fpetrogalli 
>>> wrote:
>>>
 After submitting this, I had to revert it 
 
  because of failures like 
 https://lab.llvm.org/buildbot/#/builders/225/builds/12367/steps/5/logs/stdio
>>>
>>> I have resubmitted with what I hope is the right fix (I could not reproduce 
>>> any of the failures I was seeing in buildbot, on my machine the build is 
>>> fine).
>>>
>>> The new commit is at 
>>> https://github.com/llvm/llvm-project/commit/ac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf
>>>  - in the commit message I have explained what I have changed WRT this 
>>> original patch. I have added  the
>>> tablegen target `RISCVTargetParserTableGen` in the `DEPENDS` list of 
>>> `clangDriver` and `clangBasic`. This makes sure that the `.*inc` file with 
>>> theist o the CPU is available even if `LLVMTargetParser` has not been built 
>>> yet.
>>
>> But you didn't use the proper Differential Revision tag, so the diff here 
>> didn't get updated to reflect the amended version committed :(
>
> What should I have done? Add  the `Differential Revision: 
> https://reviews.llvm.org/D137517` as the last line of the commit with the 
> rework? I wasn't aware of this for reworks.

Yes, it should have the same trailer as the original commit, otherwise it won't 
be correctly tracked by Phabricator. A "rework" isn't special, it's revert, 
reopen the revision, update the revision and land the revision again. If 
re-review isn't needed then you can skip some of the middle, but that's it. 
Though in this case I do think re-review was warranted, the new clang 
dependency seems a bit dubious and hints at the design not being quite right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

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

[PATCH] D141545: [OpenMP] Implement `omp_get_mapped_ptr`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 488391.
tianshilei1992 marked 3 inline comments as done.
tianshilei1992 added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

rebase and fix comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141545

Files:
  clang/docs/OpenMPSupport.rst
  openmp/libomptarget/src/api.cpp
  openmp/libomptarget/src/exports
  openmp/libomptarget/test/api/omp_get_mapped_ptr.c

Index: openmp/libomptarget/test/api/omp_get_mapped_ptr.c
===
--- /dev/null
+++ openmp/libomptarget/test/api/omp_get_mapped_ptr.c
@@ -0,0 +1,39 @@
+// RUN: %libomptarget-compile-and-run-generic
+
+#include 
+#include 
+#include 
+
+#define N 1024
+#define OFFSET 16
+
+int main(int argc, char *argv[]) {
+  int *host_data = (int *)malloc(sizeof(int) * N);
+  void *device_ptr = omp_get_mapped_ptr(host_data, 0);
+
+  assert(device_ptr == NULL && "the pointer should not be mapped right now");
+
+#pragma omp target enter data map(to: host_data[:N])
+
+  device_ptr = omp_get_mapped_ptr(host_data, 0);
+
+  assert(device_ptr && "the pointer should be mapped now");
+
+  void *ptr = NULL;
+
+#pragma omp target map(from: ptr)
+  { ptr = host_data; }
+
+  assert(ptr == device_ptr && "wrong pointer mapping");
+
+  device_ptr = omp_get_mapped_ptr(host_data + OFFSET, 0);
+
+  assert(device_ptr && "the pointer with offset should be mapped");
+
+#pragma omp target map(from: ptr)
+  { ptr = host_data + OFFSET; }
+
+  assert(ptr == device_ptr && "wrong pointer mapping");
+
+  return 0;
+}
Index: openmp/libomptarget/src/exports
===
--- openmp/libomptarget/src/exports
+++ openmp/libomptarget/src/exports
@@ -31,6 +31,7 @@
 __tgt_push_mapper_component;
 __kmpc_push_target_tripcount;
 __kmpc_push_target_tripcount_mapper;
+omp_get_mapped_ptr;
 omp_get_num_devices;
 omp_get_device_num;
 omp_get_initial_device;
Index: openmp/libomptarget/src/api.cpp
===
--- openmp/libomptarget/src/api.cpp
+++ openmp/libomptarget/src/api.cpp
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 EXTERN int omp_get_num_devices(void) {
   TIMESCOPE();
@@ -318,3 +319,52 @@
   DP("omp_target_disassociate_ptr returns %d\n", Rc);
   return Rc;
 }
+
+EXTERN void *omp_get_mapped_ptr(const void *Ptr, int DeviceNum) {
+  TIMESCOPE();
+  DP("Call to omp_get_mapped_ptr with ptr " DPxMOD ", device_num %d.\n",
+ DPxPTR(Ptr), DeviceNum);
+
+  if (!Ptr) {
+REPORT("Call to omp_get_mapped_ptr with nullptr.\n");
+return nullptr;
+  }
+
+  if (DeviceNum == omp_get_initial_device()) {
+REPORT("Device %d is initial device, returning Ptr " DPxMOD ".\n",
+   DeviceNum, DPxPTR(Ptr));
+return const_cast(Ptr);
+  }
+
+  int DevicesSize = omp_get_initial_device();
+  {
+std::lock_guard LG(PM->RTLsMtx);
+DevicesSize = PM->Devices.size();
+  }
+  if (DevicesSize <= DeviceNum) {
+DP("DeviceNum %d is invalid, returning nullptr.\n", DeviceNum);
+return nullptr;
+  }
+
+  if (!deviceIsReady(DeviceNum)) {
+REPORT("Device %d is not ready, returning nullptr.\n", DeviceNum);
+return nullptr;
+  }
+
+  bool IsLast = false;
+  bool IsHostPtr = false;
+  auto  = *PM->Devices[DeviceNum];
+  TargetPointerResultTy TPR =
+  Device.getTgtPtrBegin(const_cast(Ptr), 1, IsLast,
+/*UpdateRefCount=*/false,
+/*UseHoldRefCount=*/false, IsHostPtr);
+  if (!TPR.isPresent()) {
+DP("Ptr " DPxMOD "is not present on device %d, returning nullptr.\n",
+   DPxPTR(Ptr), DeviceNum);
+return nullptr;
+  }
+
+  DP("omp_get_mapped_ptr returns " DPxMOD ".\n", DPxPTR(TPR.TargetPointer));
+
+  return TPR.TargetPointer;
+}
Index: clang/docs/OpenMPSupport.rst
===
--- clang/docs/OpenMPSupport.rst
+++ clang/docs/OpenMPSupport.rst
@@ -111,7 +111,7 @@
 
 The following table provides a quick overview over various OpenMP 5.0 features
 and their implementation status. Please post on the
-`Discourse forums (Runtimes - OpenMP category)`_ for more 
+`Discourse forums (Runtimes - OpenMP category)`_ for more
 information or if you want to help with the
 implementation.
 
@@ -257,8 +257,8 @@
 
 The following table provides a quick overview over various OpenMP 5.1 features
 and their implementation status, as defined in the technical report 8 (TR8).
-Please post on the 
-`Discourse forums (Runtimes - OpenMP category)`_ for more 
+Please post on the
+`Discourse forums (Runtimes - OpenMP category)`_ for more
 information or if you want to help with the
 implementation.
 
@@ -283,7 +283,7 @@
 

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

[Francesco Petrogalli via Phabricator via cfe-commits]

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

In D137517#4045299 , @jrtc27 wrote:

> In D137517#4042875 , @fpetrogalli 
> wrote:
>
>> In D137517#4042758 , @fpetrogalli 
>> wrote:
>>
>>> After submitting this, I had to revert it 
>>> 
>>>  because of failures like 
>>> https://lab.llvm.org/buildbot/#/builders/225/builds/12367/steps/5/logs/stdio
>>
>> I have resubmitted with what I hope is the right fix (I could not reproduce 
>> any of the failures I was seeing in buildbot, on my machine the build is 
>> fine).
>>
>> The new commit is at 
>> https://github.com/llvm/llvm-project/commit/ac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf
>>  - in the commit message I have explained what I have changed WRT this 
>> original patch. I have added  the
>> tablegen target `RISCVTargetParserTableGen` in the `DEPENDS` list of 
>> `clangDriver` and `clangBasic`. This makes sure that the `.*inc` file with 
>> theist o the CPU is available even if `LLVMTargetParser` has not been built 
>> yet.
>
> But you didn't use the proper Differential Revision tag, so the diff here 
> didn't get updated to reflect the amended version committed :(

What should I have done? Add  the `Differential Revision: 
https://reviews.llvm.org/D137517` as the last line of the commit with the 
rework? I wasn't aware of this for reworks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

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

[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

[Alan Zhao via Phabricator via cfe-commits]

ayzhao added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:6198-6199
+  
dyn_cast(DestType->getAs()->getDecl());
+  S.getLangOpts().CPlusPlus20 && RD && RD->isAggregate() && Failed() &&
+  getFailureKind() == FK_ConstructorOverloadFailed &&
+  onlyHasDefaultedCtors(getFailedCandidateSet())) {

rsmith wrote:
> For when you look at re-landing this, we encountered a regression for this 
> case:
> 
> ```
> struct A {};
> struct B : A { Noncopyable nc; };
> ```
> 
> Here, given `B b`, a `B(b)` direct-initialization should be rejected because 
> it selects a deleted copy constructor of `B`, but it looks like Clang instead 
> performed aggregate initialization, as if by `B{(const A&)b, {}}`. This is 
> pretty bad -- copies that should be disallowed end up working but not 
> actually copying any members of the derived class!
> 
> The general problem is that Clang's overload resolution incorrectly handles 
> deleted functions, treating them as an overload resolution failure rather 
> than a success. In this particular case, what happens is constructor overload 
> resolution produces `OR_Deleted`, which gets mapped into 
> `FK_ConstructorOverloadFailed` despite the fact that overload resolution 
> succeeded and picked a viable function.
> 
> I guess we can split `FK_*OverloadFailed` into separate "failed" and 
> "succeeded but found a deleted function" cases?
> For when you look at re-landing this, we encountered a regression for this 
> case:
> 
> ```
> struct A {};
> struct B : A { Noncopyable nc; };
> ```
> 
> Here, given `B b`, a `B(b)` direct-initialization should be rejected because 
> it selects a deleted copy constructor of `B`, but it looks like Clang instead 
> performed aggregate initialization, as if by `B{(const A&)b, {}}`. This is 
> pretty bad -- copies that should be disallowed end up working but not 
> actually copying any members of the derived class!
> 
> The general problem is that Clang's overload resolution incorrectly handles 
> deleted functions, treating them as an overload resolution failure rather 
> than a success. In this particular case, what happens is constructor overload 
> resolution produces `OR_Deleted`, which gets mapped into 
> `FK_ConstructorOverloadFailed` despite the fact that overload resolution 
> succeeded and picked a viable function.
> 
> I guess we can split `FK_*OverloadFailed` into separate "failed" and 
> "succeeded but found a deleted function" cases?

This was the cause of https://github.com/llvm/llvm-project/issues/59675 as 
well. The reland at D141546 should handle this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129531

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

[PATCH] D141547: Fix format for `case` in .proto files

[Matt Kulukundis via Phabricator via cfe-commits]

fowles created this revision.
fowles added a reviewer: krasimir.
Herald added a project: All.
fowles requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix format for `case` in .proto files


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141547

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/test/Format/case.proto


Index: clang/test/Format/case.proto
===
--- /dev/null
+++ clang/test/Format/case.proto
@@ -0,0 +1,15 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -assume-filename=case.proto -style=Google \
+// RUN:   | FileCheck -strict-whitespace %s
+
+syntax = "proto2";
+
+package foo.bar;
+
+message Baz {
+  optional int64 fred = 1;
+// CHECK: {{^\ {2}required}}
+required string case = 2;
+// CHECK: {{^\ {2}repeated}}
+repeated int32 fizz = 1;
+}
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -590,8 +590,9 @@
   [[fallthrough]];
 }
 case tok::kw_case:
-  if (Style.isVerilog() ||
+  if (Style.isProto() || Style.isVerilog() ||
   (Style.isJavaScript() && Line->MustBeDeclaration)) {
+// Proto: there are no switch/case statements
 // Verilog: Case labels don't have this word. We handle case
 // labels including default in TokenAnnotator.
 // JavaScript: A 'case: string' style field declaration.
@@ -1620,7 +1621,11 @@
 // e.g. "default void f() {}" in a Java interface.
 break;
   case tok::kw_case:
-// In Verilog switch is called case.
+// Proto: there are no switch/case statements.
+if (Style.isProto()) {
+  nextToken();
+  return;
+}
 if (Style.isVerilog()) {
   parseBlock();
   addUnwrappedLine();
@@ -2100,6 +2105,11 @@
   parseNew();
   break;
 case tok::kw_case:
+  // Proto: there are no switch/case statements.
+  if (Style.isProto()) {
+nextToken();
+return;
+  }
   // In Verilog switch is called case.
   if (Style.isVerilog()) {
 parseBlock();


Index: clang/test/Format/case.proto
===
--- /dev/null
+++ clang/test/Format/case.proto
@@ -0,0 +1,15 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -assume-filename=case.proto -style=Google \
+// RUN:   | FileCheck -strict-whitespace %s
+
+syntax = "proto2";
+
+package foo.bar;
+
+message Baz {
+  optional int64 fred = 1;
+// CHECK: {{^\ {2}required}}
+required string case = 2;
+// CHECK: {{^\ {2}repeated}}
+repeated int32 fizz = 1;
+}
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -590,8 +590,9 @@
   [[fallthrough]];
 }
 case tok::kw_case:
-  if (Style.isVerilog() ||
+  if (Style.isProto() || Style.isVerilog() ||
   (Style.isJavaScript() && Line->MustBeDeclaration)) {
+// Proto: there are no switch/case statements
 // Verilog: Case labels don't have this word. We handle case
 // labels including default in TokenAnnotator.
 // JavaScript: A 'case: string' style field declaration.
@@ -1620,7 +1621,11 @@
 // e.g. "default void f() {}" in a Java interface.
 break;
   case tok::kw_case:
-// In Verilog switch is called case.
+// Proto: there are no switch/case statements.
+if (Style.isProto()) {
+  nextToken();
+  return;
+}
 if (Style.isVerilog()) {
   parseBlock();
   addUnwrappedLine();
@@ -2100,6 +2105,11 @@
   parseNew();
   break;
 case tok::kw_case:
+  // Proto: there are no switch/case statements.
+  if (Style.isProto()) {
+nextToken();
+return;
+  }
   // In Verilog switch is called case.
   if (Style.isVerilog()) {
 parseBlock();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D137517#4042875 , @fpetrogalli 
wrote:

> In D137517#4042758 , @fpetrogalli 
> wrote:
>
>> After submitting this, I had to revert it 
>> 
>>  because of failures like 
>> https://lab.llvm.org/buildbot/#/builders/225/builds/12367/steps/5/logs/stdio
>
> I have resubmitted with what I hope is the right fix (I could not reproduce 
> any of the failures I was seeing in buildbot, on my machine the build is 
> fine).
>
> The new commit is at 
> https://github.com/llvm/llvm-project/commit/ac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf
>  - in the commit message I have explained what I have changed WRT this 
> original patch. I have added  the
> tablegen target `RISCVTargetParserTableGen` in the `DEPENDS` list of 
> `clangDriver` and `clangBasic`. This makes sure that the `.*inc` file with 
> theist o the CPU is available even if `LLVMTargetParser` has not been built 
> yet.

But you didn't use the proper Differential Revision tag, so the diff here 
didn't get updated to reflect the amended version committed :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

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

[PATCH] D141546: [clang] Reland parenthesized aggregate init patches

[Alan Zhao via Phabricator via cfe-commits]

ayzhao created this revision.
ayzhao added a reviewer: ilya-biryukov.
Herald added subscribers: steakhal, martong, arphaman.
Herald added a reviewer: NoQ.
Herald added a project: All.
ayzhao requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This commit relands the patches for implementing P0960R3 and P1975R0,
which describe initializing aggregates via a parenthesized list.

The relanded commits are:

- 40c52159d3ee 
 - P0960R3 
and P1975R0: Allow initializing aggregates from a parenthesized list of values
- c77a91bb7ba7 
 - Remove 
overly restrictive aggregate paren init logic
- 32d7aae04fdb 
 - Fix a 
clang crash on invalid code in C++20 mode

This patch also fixes a crash in the original implementation.
Previously, if the input tried to call an implicitly deleted copy or
move constructor of a union, we would then try to initialize the union
by initializing it's first element with a reference to a union. This
behavior is incorrect (we should fail to initialize) and if the type of
the first element has a constructor with a single template typename
parameter, then Clang will explode. This patch fixes that issue by
checking that constructor overload resolution did not result in a
deleted function before attempting parenthesized aggregate
initialization.

Additionally, this patch also includes D140159 
, which contains some
minor fixes made in response to code review comments in the original
implementation that were made after that patch was submitted.

Co-authored-by: Sheng 

Fixes #54040, Fixes #59675


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141546

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/ComputeDependence.h
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Initialization.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/p4.cpp
  clang/test/CodeGen/paren-list-agg-init.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/PCH/cxx_paren_init.cpp
  clang/test/PCH/cxx_paren_init.h
  clang/test/SemaCXX/cxx2a-explicit-bool.cpp
  clang/test/SemaCXX/paren-list-agg-init.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1156,7 +1156,7 @@
 
   Parenthesized initialization of aggregates
   https://wg21.link/p0960r3;>P0960R3
-  No
+  Clang 16
 

 https://wg21.link/p1975r0;>P1975R0
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -643,6 +643,10 @@
 K = CXCursor_RequiresExpr;
 break;
 
+  case Stmt::CXXParenListInitExprClass:
+K = CXCursor_CXXParenListInitExpr;
+break;
+
   case Stmt::MSDependentExistsStmtClass:
 K = CXCursor_UnexposedStmt;
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2139,6 +2139,7 @@
   void VisitLambdaExpr(const LambdaExpr *E);
   void VisitConceptSpecializationExpr(const ConceptSpecializationExpr *E);
   void VisitRequiresExpr(const RequiresExpr *E);
+  void VisitCXXParenListInitExpr(const CXXParenListInitExpr *E);
   void VisitOMPExecutableDirective(const OMPExecutableDirective *D);
   void VisitOMPLoopBasedDirective(const OMPLoopBasedDirective *D);
   void VisitOMPLoopDirective(const OMPLoopDirective *D);
@@ -3006,6 +3007,9 

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/include/llvm/TargetParser/CMakeLists.txt:3
+tablegen(LLVM RISCVTargetParserDef.inc -gen-riscv-target-def -I 
${CMAKE_SOURCE_DIR}/lib/Target/RISCV/)
+add_public_tablegen_target(RISCVTargetParserTableGen)
+

fpetrogalli wrote:
> thakis wrote:
> > All other target tablegen'ing for all other targets (also for RISCV for 
> > other .td changes) happen in llvm/lib/Target, not in llvm/TargetParser. Is 
> > there any way this could be structured that way too? As-is, the layering 
> > looks pretty unusual. (And your reland had to teach clang about tablegen 
> > targets for that reason.)
> Thanks for the feedback.
> 
> I can think of two possible solutions:
> 
> 1. Move the RISCV-only part of `llvm/lib/TargetParser` into `llvm/lib/Target`
> 2. Move the whole TargetParser inside Target
> 
> However, I am not really sure if these are less unusual than the current 
> state of things (or even technically possible).
> 
>  I am open for suggestions, and even more open to review a solution that 
> fixes the unusual layering ;).
> 
I could be wrong, but I don't think clang depends on the LLVM's Target 
libraries today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

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

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

[Francesco Petrogalli via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/TargetParser/CMakeLists.txt:3
+tablegen(LLVM RISCVTargetParserDef.inc -gen-riscv-target-def -I 
${CMAKE_SOURCE_DIR}/lib/Target/RISCV/)
+add_public_tablegen_target(RISCVTargetParserTableGen)
+

thakis wrote:
> All other target tablegen'ing for all other targets (also for RISCV for other 
> .td changes) happen in llvm/lib/Target, not in llvm/TargetParser. Is there 
> any way this could be structured that way too? As-is, the layering looks 
> pretty unusual. (And your reland had to teach clang about tablegen targets 
> for that reason.)
Thanks for the feedback.

I can think of two possible solutions:

1. Move the RISCV-only part of `llvm/lib/TargetParser` into `llvm/lib/Target`
2. Move the whole TargetParser inside Target

However, I am not really sure if these are less unusual than the current state 
of things (or even technically possible).

 I am open for suggestions, and even more open to review a solution that fixes 
the unusual layering ;).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

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

[PATCH] D141389: [DFSAN] Add support for strnlen, strncat, strsep, sscanf and _tolower

[Andrew via Phabricator via cfe-commits]

browneee added a comment.

Please send separate changes one by one (e.g. first patch would be for `strsep` 
alone and would include several functions of implementation `__dfsw_strsep` + 
`__dfso_strsep` + test code for this).


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

https://reviews.llvm.org/D141389

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

[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:6198-6199
+  
dyn_cast(DestType->getAs()->getDecl());
+  S.getLangOpts().CPlusPlus20 && RD && RD->isAggregate() && Failed() &&
+  getFailureKind() == FK_ConstructorOverloadFailed &&
+  onlyHasDefaultedCtors(getFailedCandidateSet())) {

For when you look at re-landing this, we encountered a regression for this case:

```
struct A {};
struct B : A { Noncopyable nc; };
```

Here, given `B b`, a `B(b)` direct-initialization should be rejected because it 
selects a deleted copy constructor of `B`, but it looks like Clang instead 
performed aggregate initialization, as if by `B{(const A&)b, {}}`. This is 
pretty bad -- copies that should be disallowed end up working but not actually 
copying any members of the derived class!

The general problem is that Clang's overload resolution incorrectly handles 
deleted functions, treating them as an overload resolution failure rather than 
a success. In this particular case, what happens is constructor overload 
resolution produces `OR_Deleted`, which gets mapped into 
`FK_ConstructorOverloadFailed` despite the fact that overload resolution 
succeeded and picked a viable function.

I guess we can split `FK_*OverloadFailed` into separate "failed" and "succeeded 
but found a deleted function" cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129531

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

[PATCH] D140455: [Clang] Diagnose undefined behavior in a constant expression while evaluating a compound assignment with remainder as operand

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 488383.
shafik marked 3 inline comments as done.
shafik added a comment.

- Move HandleOverflowResult closer to where it is used.
- move int after constexpr in function rem
- Add release note


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

https://reviews.llvm.org/D140455

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/CXX/expr/expr.const/p2-0x.cpp


Index: clang/test/CXX/expr/expr.const/p2-0x.cpp
===
--- clang/test/CXX/expr/expr.const/p2-0x.cpp
+++ clang/test/CXX/expr/expr.const/p2-0x.cpp
@@ -298,6 +298,15 @@
 static_assert(isinf(f6), "");
 static_assert(isinf(f9), "");
   }
+
+#if __cplusplus >= 201703L
+namespace CompoundAssignment {
+constexpr int rem() { // expected-error {{constexpr function never produces a 
constant expression}}
+int x = ~__INT_MAX__;
+return x%=-1; // cxx20-note {{value 2147483648 is outside the range of 
representable values of type 'int'}}
+}
+}
+#endif
 }
 
 // - a lambda-expression (5.1.2);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2769,18 +2769,19 @@
   case BO_Or:  Result = LHS | RHS; return true;
   case BO_Div:
   case BO_Rem:
+bool HandleOverflowResult = true;
 if (RHS == 0) {
   Info.FFDiag(E, diag::note_expr_divide_by_zero);
   return false;
 }
-Result = (Opcode == BO_Rem ? LHS % RHS : LHS / RHS);
 // Check for overflow case: INT_MIN / -1 or INT_MIN % -1. APSInt supports
 // this operation and gives the two's complement result.
 if (RHS.isNegative() && RHS.isAllOnes() && LHS.isSigned() &&
 LHS.isMinSignedValue())
-  return HandleOverflow(Info, E, -LHS.extend(LHS.getBitWidth() + 1),
-E->getType());
-return true;
+  HandleOverflowResult = HandleOverflow(
+  Info, E, -LHS.extend(LHS.getBitWidth() + 1), E->getType());
+Result = (Opcode == BO_Rem ? LHS % RHS : LHS / RHS);
+return HandleOverflowResult;
   case BO_Shl: {
 if (Info.getLangOpts().OpenCL)
   // OpenCL 6.3j: shift values are effectively % word size of LHS.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -447,6 +447,8 @@
   ``#pragma clang __debug sloc_usage`` can also be used to request this report.
 - Clang no longer permits the keyword 'bool' in a concept declaration as a
   concepts-ts compatibility extension.
+- Clang now diagnoses overflow undefined behavior in a constant expression 
while
+  evaluating a compound assignment with remainder as operand.
 
 Non-comprehensive list of changes in this release
 -


Index: clang/test/CXX/expr/expr.const/p2-0x.cpp
===
--- clang/test/CXX/expr/expr.const/p2-0x.cpp
+++ clang/test/CXX/expr/expr.const/p2-0x.cpp
@@ -298,6 +298,15 @@
 static_assert(isinf(f6), "");
 static_assert(isinf(f9), "");
   }
+
+#if __cplusplus >= 201703L
+namespace CompoundAssignment {
+constexpr int rem() { // expected-error {{constexpr function never produces a constant expression}}
+int x = ~__INT_MAX__;
+return x%=-1; // cxx20-note {{value 2147483648 is outside the range of representable values of type 'int'}}
+}
+}
+#endif
 }
 
 // - a lambda-expression (5.1.2);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2769,18 +2769,19 @@
   case BO_Or:  Result = LHS | RHS; return true;
   case BO_Div:
   case BO_Rem:
+bool HandleOverflowResult = true;
 if (RHS == 0) {
   Info.FFDiag(E, diag::note_expr_divide_by_zero);
   return false;
 }
-Result = (Opcode == BO_Rem ? LHS % RHS : LHS / RHS);
 // Check for overflow case: INT_MIN / -1 or INT_MIN % -1. APSInt supports
 // this operation and gives the two's complement result.
 if (RHS.isNegative() && RHS.isAllOnes() && LHS.isSigned() &&
 LHS.isMinSignedValue())
-  return HandleOverflow(Info, E, -LHS.extend(LHS.getBitWidth() + 1),
-E->getType());
-return true;
+  HandleOverflowResult = HandleOverflow(
+  Info, E, -LHS.extend(LHS.getBitWidth() + 1), E->getType());
+Result = (Opcode == BO_Rem ? LHS % RHS : LHS / RHS);
+return HandleOverflowResult;
   case BO_Shl: {
 if (Info.getLangOpts().OpenCL)
   // OpenCL 6.3j: shift values are effectively % word size of LHS.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -447,6 +447,8 @@
   ``#pragma clang 

[PATCH] D141447: clang/OpenCL: Don't use a Function for the block type

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D141447#4044886 , @arsenm wrote:

> In D141447#4042273 , @yaxunl wrote:
>
>> need a test
>
> This is NFC, this is just fixing the API for the next commit I haven't posted 
> yet

Did you change the calling convention of the created kernel for amdgpu?


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

https://reviews.llvm.org/D141447

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

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D140860#4044937 , @aaron.ballman 
wrote:

> In D140860#4031872 , @dblaikie 
> wrote:
>
>> The risk now is that this might significantly regress/add new findings for 
>> this warning that may not be sufficiently bug-finding to be worth immediate 
>> cleanup, causing users to have to choose between extensive lower-value 
>> cleanup and disabling the warning entirely.
>>
>> Have you/could you run this over a significant codebase to see what sort of 
>> new findings the modified warning finds, to see if they're high quality bug 
>> finding, or mostly noise/check for whether this starts to detect certain 
>> idioms we want to handle differently?
>>
>> It might be hard to find a candidate codebase that isn't already 
>> warning-clean with GCC (at least Clang/LLVM wouldn't be a good candidate 
>> because of this) & maybe that's sufficient justification to not worry too 
>> much about this outcome...
>>
>> @aaron.ballman curious what your take on this might be
>
> Thank you for the ping (and the patience waiting on my response)!
>
> I think there's a design here that could make sense to me.
>
> Issuing the diagnostic when there is a literal is silly because the literal 
> value is never going to change. However, with a constant expression, the 
> value could change depending on configuration. This begs the question of: 
> what do we do with literals that are expanded from a macro? It looks like we 
> elide the diagnostic in that case, but macros also imply potential 
> configurability. So I think the design that would make sense to me is to 
> treat macro expansions and constant expressions the same way (diagnose) and 
> only elide the diagnostic when there's a (possibly string) literal. WDYT?

Yeah, I'm OK with that - though I also wouldn't feel strongly about ensuring we 
warn on the macro case too - if the incremental improvement to do constexpr 
values is enough for now and a note is left to let someone know they could 
expand it to handle macros.

But equally it's probably not super difficult to check if the literal is from a 
macro source location that differs from the source location of either of the 
operators, I guess? (I guess that check would be needed, so it doesn't warn 
when the macro is literally 'x && y || true' or the like.

> I would also like to see the diagnostic run over some reasonably large corpus 
> of code as it may point out cases where our reasoning is incorrect that we 
> can address. But I don't insist on it in this case given that this is also 
> coming more in line with GCC's behavior (I've not tested how they handle the 
> macro-expanding-to-a-literal case though).

Yeah, ideally.


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

https://reviews.llvm.org/D140860

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

[PATCH] D141175: [test] Split out Annotations from `TestingSupport`

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht updated this revision to Diff 488381.
rupprecht added a comment.

- Remove redundant comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141175

Files:
  clang-tools-extra/clangd/unittests/Annotations.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
  clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
  clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
  clang-tools-extra/pseudo/unittests/BracketTest.cpp
  clang-tools-extra/pseudo/unittests/CMakeLists.txt
  clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
  clang/docs/tools/clang-formatted-files.txt
  clang/unittests/AST/CMakeLists.txt
  clang/unittests/AST/DeclTest.cpp
  clang/unittests/AST/SourceLocationTest.cpp
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
  clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferBranchTest.cpp
  clang/unittests/Sema/CMakeLists.txt
  clang/unittests/Sema/CodeCompleteTest.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/SourceCodeTest.cpp
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TokensTest.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.h
  llvm/include/llvm/Testing/Annotations/Annotations.h
  llvm/include/llvm/Testing/Support/Annotations.h
  llvm/lib/Testing/Annotations/Annotations.cpp
  llvm/lib/Testing/Annotations/CMakeLists.txt
  llvm/lib/Testing/CMakeLists.txt
  llvm/lib/Testing/Support/Annotations.cpp
  llvm/lib/Testing/Support/CMakeLists.txt
  llvm/unittests/Support/AnnotationsTest.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Testing/Annotations/AnnotationsTest.cpp
  llvm/unittests/Testing/Annotations/CMakeLists.txt
  llvm/unittests/Testing/CMakeLists.txt
  utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

Index: utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
===
--- utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
+++ utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
@@ -4450,18 +4450,26 @@
 "lib/Testing/Support/*.cpp",
 "lib/Testing/Support/*.h",
 ]),
-hdrs = glob([
-"include/llvm/Testing/Support/*.h",
-]),
+hdrs = glob(["include/llvm/Testing/Support/*.h"]),
 copts = llvm_copts,
 deps = [
 ":Support",
+":TestingAnnotations",
 ":config",
 "//third-party/unittest:gmock",
 "//third-party/unittest:gtest",
 ],
 )
 
+cc_library(
+name = "TestingAnnotations",
+testonly = True,
+srcs = ["lib/Testing/Annotations/Annotations.cpp"],
+hdrs = ["include/llvm/Testing/Annotations/Annotations.h"],
+copts = llvm_copts,
+deps = [":Support"],
+)
+
 
 # Begin testonly binary utilities
 
Index: llvm/unittests/Testing/CMakeLists.txt
===
--- llvm/unittests/Testing/CMakeLists.txt
+++ llvm/unittests/Testing/CMakeLists.txt
@@ -1,2 +1,3 @@
 add_subdirectory(ADT)
+add_subdirectory(Annotations)
 add_subdirectory(Support)
Index: llvm/unittests/Testing/Annotations/CMakeLists.txt
===
--- /dev/null
+++ llvm/unittests/Testing/Annotations/CMakeLists.txt
@@ -0,0 +1,10 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  TestingAnnotations
+  )
+
+add_llvm_unittest(TestingAnnotationTests
+  AnnotationsTest.cpp
+  )
+
+target_link_libraries(TestingAnnotationTests PRIVATE LLVMTestingAnnotations)
Index: llvm/unittests/Testing/Annotations/AnnotationsTest.cpp
===
--- llvm/unittests/Testing/Annotations/AnnotationsTest.cpp
+++ llvm/unittests/Testing/Annotations/AnnotationsTest.cpp
@@ -5,7 +5,7 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
-#include "llvm/Testing/Support/Annotations.h"
+#include "llvm/Testing/Annotations/Annotations.h"
 #include "gmock/gmock.h"
 

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

That's about what I would expect.  One or two extra instructions per argument 
that are trivially removed in debug builds.

I don't really see a need to post about this on Discourse, but it might be 
worth a release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141381

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

hmm, did we/do we ever motivate backend diagnostics using the debug info 
locations? I thought we did in some cases/ways - maybe when the frontend isn't 
available (eg: when compiling from LLVM IR).

It'd be nice not to invent a new way of tracking inlining separate from the way 
debug info does this - duplicate systems with separate opportunities for bugs, 
etc. Any chance we can reuse the debug info inlining descriptions for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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

[PATCH] D141092: Optionally install clang-tblgen to aid cross-compiling

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

Can you please share the `Name ` to use for the commit?


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

https://reviews.llvm.org/D141092

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:2467-2468
+
+const Function *Callee = CI->getCalledFunction();
+if (Callee && (Callee->hasFnAttribute("dontcall-error") ||
+   Callee->hasFnAttribute("dontcall-warn"))) {

arsenm wrote:
> Misses constexpr casts and aliases
The base feature doesn't work with aliases (or ConstantExpr), in GCC or Clang.  
I should perhaps fix that first...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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

[PATCH] D141467: [clang][Driver][CUDA] Get rid of unused LibPath

[Artem Belevich via Phabricator via cfe-commits]

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

Given that it's indeed unused, I'm fine with removing it.

That said, it's somewhat odd that in your setup clang was able to find 
everything but the library directory. You generally would need to have 
`--cuda-path=` among compiler options and that should be pointing to a valid 
CUDA installation.

Can you elaborate on where/how clang manages to find CUDA headers in your case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141467

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

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5736
+  C.getArgs().hasArg(options::OPT_fmodule_output) &&
+  C.getArgs().hasArg(options::OPT_o)) {
+SmallString<128> OutputPath;

dblaikie wrote:
> ChuanqiXu wrote:
> > dblaikie wrote:
> > > Is there some way we can avoid this (`-fmodule-output  -o ...`) being a 
> > > special case? It'd be good if we could use a single common implementation 
> > > regardless of whether `-o` is used (ie: Figure out the output file name 
> > > (whether it's implicitly determined by clang, in the absence of `-o`, or 
> > > explicitly provided by the user through `-o`) and then replace the suffix 
> > > with `pcm`)?
> > I guess we can't do it or we can't do it easily. Otherwise the .pcm file 
> > will always lives in the same directory with the input file.
> > I guess we can't do it or we can't do it easily. Otherwise the .pcm file 
> > will always lives in the same directory with the input file.
> 
> I don't follow/understand. What I mean is, I'd like it, if possible, this was 
> implemented by something like "find the path for the .o file output, then 
> replace the extension with .pcm".
> 
> I worry atht code like this that recomputes something similar to the object 
> output path risks getting out of sync with the actual object path.
That is certainly a valid concern and I agree it would be better if the shared 
output path is computed exactly once. If that would require significant change, 
then tests to ensure consistent behavior would be the next best thing. I'm not 
sure what infrastructure is in place for validation of output file locations 
though.


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

https://reviews.llvm.org/D137058

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

[PATCH] D141092: Optionally install clang-tblgen to aid cross-compiling

[James Le Cuirot via Phabricator via cfe-commits]

chewi added a comment.

I don't have commit access. Please merge this for me.


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

https://reviews.llvm.org/D141092

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

[PATCH] D141175: [test] Split out Annotations from `TestingSupport`

[Geoffrey Martin-Noble via Phabricator via cfe-commits]

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

LGTM (+/- nits), but maybe good to get review from someone more closely 
associated with the TestingSupport library




Comment at: utils/bazel/llvm-project-overlay/llvm/BUILD.bazel:4464
 
+# Split out of TestingSupport to allow using this with a different gmock/gtest 
version.
+cc_library(

I think this comment is now a bit redundant


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141175

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 488371.
nickdesaulniers added a comment.

- mention inlined.from in LangRef


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Frontend/backend-attribute-error-warning-optimize.c
  clang/test/Frontend/backend-attribute-error-warning.c
  clang/test/Frontend/backend-attribute-error-warning.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/Transforms/Inline/dontcall-attributes.ll

Index: llvm/test/Transforms/Inline/dontcall-attributes.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/dontcall-attributes.ll
@@ -0,0 +1,84 @@
+; RUN: opt -S -o - -passes=inline %s \
+; RUN:  | FileCheck %s --check-prefixes=CHECK-BOTH,CHECK
+; RUN: opt -S -o - -passes=always-inline %s \
+; RUN:  | FileCheck %s --check-prefixes=CHECK-BOTH,CHECK-ALWAYS
+
+declare void @foo() "dontcall-warn"="oh no"
+declare void @fof() "dontcall-error"="oh no"
+
+define void @bar(i32 %x) {
+  %cmp = icmp eq i32 %x, 10
+  br i1 %cmp, label %if.then, label %if.end
+
+if.then:
+  call void @foo()
+  br label %if.end
+
+if.end:
+  ret void
+}
+
+define void @quux() {
+  call void @bar(i32 9)
+  ret void
+}
+
+; Test that @baz's call to @foo has metadata with inlining info.
+define void @baz() {
+; CHECK-LABEL: @baz(
+; CHECK-NEXT:call void @foo(), !inlined.from !0
+;
+  call void @bar(i32 10)
+  ret void
+}
+
+; Test that @zing's call to @foo has unique metadata from @baz's call to @foo.
+define void @zing() {
+; CHECK-LABEL: @zing(
+; CHECK-NEXT:call void @foo(), !inlined.from !1
+;
+  call void @baz()
+  ret void
+}
+
+; Same test but @fof has fn attr "dontcall-error"="..." rather than
+; "dontcall-warn"="...".
+define void @a() {
+  call void @fof()
+  ret void
+}
+define void @b() {
+; CHECK-LABEL: @b(
+; CHECK-NEXT: call void @fof(), !inlined.from !3
+  call void @a()
+  ret void
+}
+
+; Add some tests for alwaysinline.
+define void @always_callee() alwaysinline {
+  call void @fof()
+  ret void
+}
+define void @always_caller() alwaysinline {
+; CHECK-BOTH-LABEL: @always_caller(
+; CHECK-NEXT: call void @fof(), !inlined.from !4
+; CHECK-ALWAYS-NEXT: call void @fof(), !inlined.from !0
+  call void @always_callee()
+  ret void
+}
+define void @always_caller2() alwaysinline {
+; CHECK-BOTH-LABEL: @always_caller2(
+; CHECK-NEXT: call void @fof(), !inlined.from !5
+; CHECK-ALWAYS-NEXT: call void @fof(), !inlined.from !1
+  call void @always_caller()
+  ret void
+}
+
+; CHECK: !0 = !{!"bar"}
+; CHECK-NEXT: !1 = !{!2}
+; CHECK-NEXT: !2 = !{!"bar", !"baz"}
+; CHECK-NEXT: !3 = !{!"a"}
+; CHECK-NEXT: !4 = !{!"always_callee"}
+; CHECK-ALWAYS: !0 = !{!"always_callee"}
+; CHECK-ALWAYS-NEXT: !1 = !{!2}
+; CHECK-ALWAYS-NEXT: !2 = !{!"always_callee", !"always_caller"}
Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -2463,6 +2463,19 @@
 // inlining, commonly when the callee is an intrinsic.
 if (MarkNoUnwind && !CI->doesNotThrow())
   CI->setDoesNotThrow();
+
+const Function *Callee = CI->getCalledFunction();
+if (Callee && (Callee->hasFnAttribute("dontcall-error") ||
+   Callee->hasFnAttribute("dontcall-warn"))) {
+  Metadata *MD = MDString::get(CI->getContext(), CalledFunc->getName());
+  if (MDNode *N = CI->getMetadata("inlined.from")) {
+TempMDTuple Temp = cast(N)->clone();
+Temp->push_back(MD);
+MD = MDNode::replaceWithUniqued(std::move(Temp));
+  }
+  MDTuple *MDT = MDNode::get(CI->getContext(), {MD});
+  CI->setMetadata("inlined.from", MDT);
+}
   }
 }
   }
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -12,9 +12,11 @@
 //===--===//
 
 #include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfoMetadata.h"
@@ -430,8 +432,9 @@
   if (MDNode *MD = CI.getMetadata("srcloc"))
 LocCookie =
 mdconst::extract(MD->getOperand(0))->getZExtValue();
-  DiagnosticInfoDontCall D(F->getName(), A.getValueAsString(), Sev,
-   LocCookie);
+ 

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 488368.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.

- replace inlined-from w/ inlined.from


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Frontend/backend-attribute-error-warning-optimize.c
  clang/test/Frontend/backend-attribute-error-warning.c
  clang/test/Frontend/backend-attribute-error-warning.cpp
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/Transforms/Inline/dontcall-attributes.ll

Index: llvm/test/Transforms/Inline/dontcall-attributes.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/dontcall-attributes.ll
@@ -0,0 +1,84 @@
+; RUN: opt -S -o - -passes=inline %s \
+; RUN:  | FileCheck %s --check-prefixes=CHECK-BOTH,CHECK
+; RUN: opt -S -o - -passes=always-inline %s \
+; RUN:  | FileCheck %s --check-prefixes=CHECK-BOTH,CHECK-ALWAYS
+
+declare void @foo() "dontcall-warn"="oh no"
+declare void @fof() "dontcall-error"="oh no"
+
+define void @bar(i32 %x) {
+  %cmp = icmp eq i32 %x, 10
+  br i1 %cmp, label %if.then, label %if.end
+
+if.then:
+  call void @foo()
+  br label %if.end
+
+if.end:
+  ret void
+}
+
+define void @quux() {
+  call void @bar(i32 9)
+  ret void
+}
+
+; Test that @baz's call to @foo has metadata with inlining info.
+define void @baz() {
+; CHECK-LABEL: @baz(
+; CHECK-NEXT:call void @foo(), !inlined.from !0
+;
+  call void @bar(i32 10)
+  ret void
+}
+
+; Test that @zing's call to @foo has unique metadata from @baz's call to @foo.
+define void @zing() {
+; CHECK-LABEL: @zing(
+; CHECK-NEXT:call void @foo(), !inlined.from !1
+;
+  call void @baz()
+  ret void
+}
+
+; Same test but @fof has fn attr "dontcall-error"="..." rather than
+; "dontcall-warn"="...".
+define void @a() {
+  call void @fof()
+  ret void
+}
+define void @b() {
+; CHECK-LABEL: @b(
+; CHECK-NEXT: call void @fof(), !inlined.from !3
+  call void @a()
+  ret void
+}
+
+; Add some tests for alwaysinline.
+define void @always_callee() alwaysinline {
+  call void @fof()
+  ret void
+}
+define void @always_caller() alwaysinline {
+; CHECK-BOTH-LABEL: @always_caller(
+; CHECK-NEXT: call void @fof(), !inlined.from !4
+; CHECK-ALWAYS-NEXT: call void @fof(), !inlined.from !0
+  call void @always_callee()
+  ret void
+}
+define void @always_caller2() alwaysinline {
+; CHECK-BOTH-LABEL: @always_caller2(
+; CHECK-NEXT: call void @fof(), !inlined.from !5
+; CHECK-ALWAYS-NEXT: call void @fof(), !inlined.from !1
+  call void @always_caller()
+  ret void
+}
+
+; CHECK: !0 = !{!"bar"}
+; CHECK-NEXT: !1 = !{!2}
+; CHECK-NEXT: !2 = !{!"bar", !"baz"}
+; CHECK-NEXT: !3 = !{!"a"}
+; CHECK-NEXT: !4 = !{!"always_callee"}
+; CHECK-ALWAYS: !0 = !{!"always_callee"}
+; CHECK-ALWAYS-NEXT: !1 = !{!2}
+; CHECK-ALWAYS-NEXT: !2 = !{!"always_callee", !"always_caller"}
Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -2463,6 +2463,19 @@
 // inlining, commonly when the callee is an intrinsic.
 if (MarkNoUnwind && !CI->doesNotThrow())
   CI->setDoesNotThrow();
+
+const Function *Callee = CI->getCalledFunction();
+if (Callee && (Callee->hasFnAttribute("dontcall-error") ||
+   Callee->hasFnAttribute("dontcall-warn"))) {
+  Metadata *MD = MDString::get(CI->getContext(), CalledFunc->getName());
+  if (MDNode *N = CI->getMetadata("inlined.from")) {
+TempMDTuple Temp = cast(N)->clone();
+Temp->push_back(MD);
+MD = MDNode::replaceWithUniqued(std::move(Temp));
+  }
+  MDTuple *MDT = MDNode::get(CI->getContext(), {MD});
+  CI->setMetadata("inlined.from", MDT);
+}
   }
 }
   }
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -12,9 +12,11 @@
 //===--===//
 
 #include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfoMetadata.h"
@@ -430,8 +432,9 @@
   if (MDNode *MD = CI.getMetadata("srcloc"))
 LocCookie =
 mdconst::extract(MD->getOperand(0))->getZExtValue();
-  DiagnosticInfoDontCall D(F->getName(), A.getValueAsString(), Sev,
-  

[PATCH] D141175: [bazel] Split out Annotations from `TestingSupport`

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

In D141175#4038017 , @GMNGeoffrey 
wrote:

> It seems like the same logic would extend to the CMake build. Could we make 
> the same change there?

The simplest (only?) way to do that is to have it literally in a separate 
directory, so I did that. It's a bit large now but mechanical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141175

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

[PATCH] D140584: [Clang] Refactor "Designators" into a unified implementation [NFC]

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Sonar ping. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140584

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

[PATCH] D141175: [bazel] Split out Annotations from `TestingSupport`

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht updated this revision to Diff 488366.
rupprecht added a comment.
Herald added subscribers: cfe-commits, kadircet, arphaman, hiraditya.
Herald added projects: clang, clang-tools-extra.

- Move annotations to a separate package entirely


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141175

Files:
  clang-tools-extra/clangd/unittests/Annotations.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
  clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
  clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
  clang-tools-extra/pseudo/unittests/BracketTest.cpp
  clang-tools-extra/pseudo/unittests/CMakeLists.txt
  clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
  clang/docs/tools/clang-formatted-files.txt
  clang/unittests/AST/CMakeLists.txt
  clang/unittests/AST/DeclTest.cpp
  clang/unittests/AST/SourceLocationTest.cpp
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
  clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferBranchTest.cpp
  clang/unittests/Sema/CMakeLists.txt
  clang/unittests/Sema/CodeCompleteTest.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/SourceCodeTest.cpp
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TokensTest.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.h
  llvm/include/llvm/Testing/Annotations/Annotations.h
  llvm/include/llvm/Testing/Support/Annotations.h
  llvm/lib/Testing/Annotations/Annotations.cpp
  llvm/lib/Testing/Annotations/CMakeLists.txt
  llvm/lib/Testing/CMakeLists.txt
  llvm/lib/Testing/Support/Annotations.cpp
  llvm/lib/Testing/Support/CMakeLists.txt
  llvm/unittests/Support/AnnotationsTest.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Testing/Annotations/AnnotationsTest.cpp
  llvm/unittests/Testing/Annotations/CMakeLists.txt
  llvm/unittests/Testing/CMakeLists.txt
  utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

Index: utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
===
--- utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
+++ utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
@@ -4450,18 +4450,27 @@
 "lib/Testing/Support/*.cpp",
 "lib/Testing/Support/*.h",
 ]),
-hdrs = glob([
-"include/llvm/Testing/Support/*.h",
-]),
+hdrs = glob(["include/llvm/Testing/Support/*.h"]),
 copts = llvm_copts,
 deps = [
 ":Support",
+":TestingAnnotations",
 ":config",
 "//third-party/unittest:gmock",
 "//third-party/unittest:gtest",
 ],
 )
 
+# Split out of TestingSupport to allow using this with a different gmock/gtest version.
+cc_library(
+name = "TestingAnnotations",
+testonly = True,
+srcs = ["lib/Testing/Annotations/Annotations.cpp"],
+hdrs = ["include/llvm/Testing/Annotations/Annotations.h"],
+copts = llvm_copts,
+deps = [":Support"],
+)
+
 
 # Begin testonly binary utilities
 
Index: llvm/unittests/Testing/CMakeLists.txt
===
--- llvm/unittests/Testing/CMakeLists.txt
+++ llvm/unittests/Testing/CMakeLists.txt
@@ -1,2 +1,3 @@
 add_subdirectory(ADT)
+add_subdirectory(Annotations)
 add_subdirectory(Support)
Index: llvm/unittests/Testing/Annotations/CMakeLists.txt
===
--- /dev/null
+++ llvm/unittests/Testing/Annotations/CMakeLists.txt
@@ -0,0 +1,10 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  TestingAnnotations
+  )
+
+add_llvm_unittest(TestingAnnotationTests
+  AnnotationsTest.cpp
+  )
+
+target_link_libraries(TestingAnnotationTests PRIVATE LLVMTestingAnnotations)
Index: llvm/unittests/Testing/Annotations/AnnotationsTest.cpp
===
--- llvm/unittests/Testing/Annotations/AnnotationsTest.cpp
+++ llvm/unittests/Testing/Annotations/AnnotationsTest.cpp
@@ -5,7 +5,7 @@
 // SPDX-License-Identifier: Apache-2.0