Skip to content

[mlir] Add [[lifetimebound]] to Range classes. #123091

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jan 20, 2025
Merged

[mlir] Add [[lifetimebound]] to Range classes. #123091

merged 5 commits into from
Jan 20, 2025

Conversation

chsigg
Copy link
Contributor

@chsigg chsigg commented Jan 15, 2025

This prevents creating range class instances from temporaries.

@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Jan 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 15, 2025

@llvm/pr-subscribers-mlir-core

Author: Christian Sigg (chsigg)

Changes

This prevents creating range class instances from temporaries.

Full diff: https://github.com/llvm/llvm-project/pull/123091.diff

4 Files Affected:

  • (modified) mlir/include/mlir/IR/BlockSupport.h (+4-4)
  • (modified) mlir/include/mlir/IR/Region.h (+2-2)
  • (modified) mlir/include/mlir/IR/TypeRange.h (+3-2)
  • (modified) mlir/include/mlir/IR/ValueRange.h (+5-3)
diff --git a/mlir/include/mlir/IR/BlockSupport.h b/mlir/include/mlir/IR/BlockSupport.h
index ff508891ac2ffc..b28ba9b03d0ebf 100644
--- a/mlir/include/mlir/IR/BlockSupport.h
+++ b/mlir/include/mlir/IR/BlockSupport.h
@@ -74,8 +74,8 @@ class SuccessorRange final
 public:
   using RangeBaseT::RangeBaseT;
   SuccessorRange();
-  SuccessorRange(Block *block);
-  SuccessorRange(Operation *term);
+  SuccessorRange(Block *block LLVM_LIFETIME_BOUND);
+  SuccessorRange(Operation *term LLVM_LIFETIME_BOUND);
 
 private:
   /// See `llvm::detail::indexed_accessor_range_base` for details.
@@ -110,9 +110,9 @@ class BlockRange final
   BlockRange(SuccessorRange successors);
   template <typename Arg, typename = std::enable_if_t<std::is_constructible<
                               ArrayRef<Block *>, Arg>::value>>
-  BlockRange(Arg &&arg)
+  BlockRange(Arg &&arg LLVM_LIFETIME_BOUND)
       : BlockRange(ArrayRef<Block *>(std::forward<Arg>(arg))) {}
-  BlockRange(std::initializer_list<Block *> blocks)
+  BlockRange(std::initializer_list<Block *> blocks LLVM_LIFETIME_BOUND)
       : BlockRange(ArrayRef<Block *>(blocks)) {}
 
 private:
diff --git a/mlir/include/mlir/IR/Region.h b/mlir/include/mlir/IR/Region.h
index 93fc9dbb430eec..22cb7037772ddb 100644
--- a/mlir/include/mlir/IR/Region.h
+++ b/mlir/include/mlir/IR/Region.h
@@ -357,12 +357,12 @@ class RegionRange
 
   template <typename Arg, typename = std::enable_if_t<std::is_constructible<
                               ArrayRef<std::unique_ptr<Region>>, Arg>::value>>
-  RegionRange(Arg &&arg)
+  RegionRange(Arg &&arg LLVM_LIFETIME_BOUND)
       : RegionRange(ArrayRef<std::unique_ptr<Region>>(std::forward<Arg>(arg))) {
   }
   template <typename Arg>
   RegionRange(
-      Arg &&arg,
+      Arg &&arg LLVM_LIFETIME_BOUND,
       std::enable_if_t<std::is_constructible<ArrayRef<Region *>, Arg>::value>
           * = nullptr)
       : RegionRange(ArrayRef<Region *>(std::forward<Arg>(arg))) {}
diff --git a/mlir/include/mlir/IR/TypeRange.h b/mlir/include/mlir/IR/TypeRange.h
index 99fabab334f922..9c2fbb3884188e 100644
--- a/mlir/include/mlir/IR/TypeRange.h
+++ b/mlir/include/mlir/IR/TypeRange.h
@@ -46,8 +46,9 @@ class TypeRange : public llvm::detail::indexed_accessor_range_base<
                                          values.end().getCurrent()))) {}
   template <typename Arg, typename = std::enable_if_t<std::is_constructible<
                               ArrayRef<Type>, Arg>::value>>
-  TypeRange(Arg &&arg) : TypeRange(ArrayRef<Type>(std::forward<Arg>(arg))) {}
-  TypeRange(std::initializer_list<Type> types)
+  TypeRange(Arg &&arg LLVM_LIFETIME_BOUND)
+      : TypeRange(ArrayRef<Type>(std::forward<Arg>(arg))) {}
+  TypeRange(std::initializer_list<Type> types LLVM_LIFETIME_BOUND)
       : TypeRange(ArrayRef<Type>(types)) {}
 
 private:
diff --git a/mlir/include/mlir/IR/ValueRange.h b/mlir/include/mlir/IR/ValueRange.h
index 4b421c08d8418e..a807b77ad077f8 100644
--- a/mlir/include/mlir/IR/ValueRange.h
+++ b/mlir/include/mlir/IR/ValueRange.h
@@ -391,9 +391,11 @@ class ValueRange final
             typename = std::enable_if_t<
                 std::is_constructible<ArrayRef<Value>, Arg>::value &&
                 !std::is_convertible<Arg, Value>::value>>
-  ValueRange(Arg &&arg) : ValueRange(ArrayRef<Value>(std::forward<Arg>(arg))) {}
-  ValueRange(const Value &value) : ValueRange(&value, /*count=*/1) {}
-  ValueRange(const std::initializer_list<Value> &values)
+  ValueRange(Arg &&arg LLVM_LIFETIME_BOUND)
+      : ValueRange(ArrayRef<Value>(std::forward<Arg>(arg))) {}
+  ValueRange(const Value &value LLVM_LIFETIME_BOUND)
+      : ValueRange(&value, /*count=*/1) {}
+  ValueRange(const std::initializer_list<Value> &values LLVM_LIFETIME_BOUND)
       : ValueRange(ArrayRef<Value>(values)) {}
   ValueRange(iterator_range<OperandRange::iterator> values)
       : ValueRange(OperandRange(values)) {}

@llvmbot
Copy link
Member

llvmbot commented Jan 15, 2025

@llvm/pr-subscribers-mlir

Author: Christian Sigg (chsigg)

Changes

This prevents creating range class instances from temporaries.

Full diff: https://github.com/llvm/llvm-project/pull/123091.diff

4 Files Affected:

  • (modified) mlir/include/mlir/IR/BlockSupport.h (+4-4)
  • (modified) mlir/include/mlir/IR/Region.h (+2-2)
  • (modified) mlir/include/mlir/IR/TypeRange.h (+3-2)
  • (modified) mlir/include/mlir/IR/ValueRange.h (+5-3)
diff --git a/mlir/include/mlir/IR/BlockSupport.h b/mlir/include/mlir/IR/BlockSupport.h
index ff508891ac2ffc..b28ba9b03d0ebf 100644
--- a/mlir/include/mlir/IR/BlockSupport.h
+++ b/mlir/include/mlir/IR/BlockSupport.h
@@ -74,8 +74,8 @@ class SuccessorRange final
 public:
   using RangeBaseT::RangeBaseT;
   SuccessorRange();
-  SuccessorRange(Block *block);
-  SuccessorRange(Operation *term);
+  SuccessorRange(Block *block LLVM_LIFETIME_BOUND);
+  SuccessorRange(Operation *term LLVM_LIFETIME_BOUND);
 
 private:
   /// See `llvm::detail::indexed_accessor_range_base` for details.
@@ -110,9 +110,9 @@ class BlockRange final
   BlockRange(SuccessorRange successors);
   template <typename Arg, typename = std::enable_if_t<std::is_constructible<
                               ArrayRef<Block *>, Arg>::value>>
-  BlockRange(Arg &&arg)
+  BlockRange(Arg &&arg LLVM_LIFETIME_BOUND)
       : BlockRange(ArrayRef<Block *>(std::forward<Arg>(arg))) {}
-  BlockRange(std::initializer_list<Block *> blocks)
+  BlockRange(std::initializer_list<Block *> blocks LLVM_LIFETIME_BOUND)
       : BlockRange(ArrayRef<Block *>(blocks)) {}
 
 private:
diff --git a/mlir/include/mlir/IR/Region.h b/mlir/include/mlir/IR/Region.h
index 93fc9dbb430eec..22cb7037772ddb 100644
--- a/mlir/include/mlir/IR/Region.h
+++ b/mlir/include/mlir/IR/Region.h
@@ -357,12 +357,12 @@ class RegionRange
 
   template <typename Arg, typename = std::enable_if_t<std::is_constructible<
                               ArrayRef<std::unique_ptr<Region>>, Arg>::value>>
-  RegionRange(Arg &&arg)
+  RegionRange(Arg &&arg LLVM_LIFETIME_BOUND)
       : RegionRange(ArrayRef<std::unique_ptr<Region>>(std::forward<Arg>(arg))) {
   }
   template <typename Arg>
   RegionRange(
-      Arg &&arg,
+      Arg &&arg LLVM_LIFETIME_BOUND,
       std::enable_if_t<std::is_constructible<ArrayRef<Region *>, Arg>::value>
           * = nullptr)
       : RegionRange(ArrayRef<Region *>(std::forward<Arg>(arg))) {}
diff --git a/mlir/include/mlir/IR/TypeRange.h b/mlir/include/mlir/IR/TypeRange.h
index 99fabab334f922..9c2fbb3884188e 100644
--- a/mlir/include/mlir/IR/TypeRange.h
+++ b/mlir/include/mlir/IR/TypeRange.h
@@ -46,8 +46,9 @@ class TypeRange : public llvm::detail::indexed_accessor_range_base<
                                          values.end().getCurrent()))) {}
   template <typename Arg, typename = std::enable_if_t<std::is_constructible<
                               ArrayRef<Type>, Arg>::value>>
-  TypeRange(Arg &&arg) : TypeRange(ArrayRef<Type>(std::forward<Arg>(arg))) {}
-  TypeRange(std::initializer_list<Type> types)
+  TypeRange(Arg &&arg LLVM_LIFETIME_BOUND)
+      : TypeRange(ArrayRef<Type>(std::forward<Arg>(arg))) {}
+  TypeRange(std::initializer_list<Type> types LLVM_LIFETIME_BOUND)
       : TypeRange(ArrayRef<Type>(types)) {}
 
 private:
diff --git a/mlir/include/mlir/IR/ValueRange.h b/mlir/include/mlir/IR/ValueRange.h
index 4b421c08d8418e..a807b77ad077f8 100644
--- a/mlir/include/mlir/IR/ValueRange.h
+++ b/mlir/include/mlir/IR/ValueRange.h
@@ -391,9 +391,11 @@ class ValueRange final
             typename = std::enable_if_t<
                 std::is_constructible<ArrayRef<Value>, Arg>::value &&
                 !std::is_convertible<Arg, Value>::value>>
-  ValueRange(Arg &&arg) : ValueRange(ArrayRef<Value>(std::forward<Arg>(arg))) {}
-  ValueRange(const Value &value) : ValueRange(&value, /*count=*/1) {}
-  ValueRange(const std::initializer_list<Value> &values)
+  ValueRange(Arg &&arg LLVM_LIFETIME_BOUND)
+      : ValueRange(ArrayRef<Value>(std::forward<Arg>(arg))) {}
+  ValueRange(const Value &value LLVM_LIFETIME_BOUND)
+      : ValueRange(&value, /*count=*/1) {}
+  ValueRange(const std::initializer_list<Value> &values LLVM_LIFETIME_BOUND)
       : ValueRange(ArrayRef<Value>(values)) {}
   ValueRange(iterator_range<OperandRange::iterator> values)
       : ValueRange(OperandRange(values)) {}

River707
River707 approved these changes Jan 15, 2025
View reviewed changes
mlir/include/mlir/IR/BlockSupport.h Outdated
@@ -74,8 +74,8 @@ class SuccessorRange final
public:
using RangeBaseT::RangeBaseT;
SuccessorRange();
SuccessorRange(Block *block);
SuccessorRange(Operation *term);
SuccessorRange(Block *block LLVM_LIFETIME_BOUND);
Copy link
Contributor

@River707 River707 Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the value of adding it here? The address of these aren't taken, and these aren't really temporaries either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks River for noticing! Removed.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 15, 2025 •
edited
Loading

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command: 
git-clang-format --diff 0e13ce770bfbee7cfbc8086a038a950fe12c03d5 96d3695b05b3e59c77a0f226d994ab28b7c891a7 --extensions h -- mlir/include/mlir/IR/BlockSupport.h mlir/include/mlir/IR/Region.h mlir/include/mlir/IR/TypeRange.h mlir/include/mlir/IR/ValueRange.h
View the diff from clang-format here. 
diff --git a/mlir/include/mlir/IR/TypeRange.h b/mlir/include/mlir/IR/TypeRange.h
index 9ee97cfe42..9c2fbb3884 100644
--- a/mlir/include/mlir/IR/TypeRange.h
+++ b/mlir/include/mlir/IR/TypeRange.h
@@ -46,7 +46,7 @@ public:
                                          values.end().getCurrent()))) {}
   template <typename Arg, typename = std::enable_if_t<std::is_constructible<
                               ArrayRef<Type>, Arg>::value>>
-  TypeRange(Arg &&arg LLVM_LIFETIME_BOUND) 
+  TypeRange(Arg &&arg LLVM_LIFETIME_BOUND)
       : TypeRange(ArrayRef<Type>(std::forward<Arg>(arg))) {}
   TypeRange(std::initializer_list<Type> types LLVM_LIFETIME_BOUND)
       : TypeRange(ArrayRef<Type>(types)) {}

@chsigg chsigg merged commit 1297c11 into llvm:main Jan 20, 2025
5 of 7 checks passed
@chsigg chsigg deleted the piper_export_cl_715815096 branch January 20, 2025 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@River707 River707 River707 approved these changes

Assignees
No one assigned
Labels
mlir:core MLIR Core Infrastructure mlir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chsigg @llvmbot @River707