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

[Piotr Zegar via Phabricator via cfe-commits]

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

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

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

[Shivam Gupta via Phabricator via cfe-commits]

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

LGTM, Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Piotr Zegar via Phabricator via cfe-commits]

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

Review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



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

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



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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Shivam Gupta via Phabricator via cfe-commits]

xgupta added inline comments.



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

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



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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Piotr Zegar via Phabricator via cfe-commits]

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

Rebase + Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Piotr Zegar via Phabricator via cfe-commits]

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

Marked check as one that provides fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Ready for review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 515475.
PiotrZSL marked an inline comment as done.
PiotrZSL added a comment.

Added support for ->, added more tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

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

[Piotr Zegar via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp:16
+  };
+}
+

njames93 wrote:
> PiotrZSL wrote:
> > njames93 wrote:
> > > It'd be good to have tests demonstating `boost::optional` and 
> > > `absl::optional` as well as the (non standard) get accessor
> > I added them to config just to add them, as they should work, I could add 
> > some tests for them, but behavior should be same...
> On the subject of boost(unsure about absl as I've never used that library)
> Would you ever add support for `T& boost::get(boost::optional &)` or its 
> const ref cousin?
No, I'm not planing to add support for boost::get. I added basic support for 
boost and absl only because it's cheap, and interfaces are similar. Main target 
is std::optional. And configuration is provider just because for example in 
project that I work for we got custom optional implementation. 
Added info about limited support for non-standard optional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL planned changes to this revision.
PiotrZSL added a comment.

TODO: Fix pointers, Add more tests, consider free standing value extraction 
functions like boost::get


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 512559.
PiotrZSL marked 3 inline comments as done.
PiotrZSL added a comment.

Add fixes, add suport for std:move, fixed operator*() calls, added more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
@@ -0,0 +1,111 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-optional-value-conversion %t -- --fix-notes
+
+namespace std {
+  template
+  struct optional
+  {
+  constexpr optional() noexcept;
+  constexpr optional(T&&) noexcept;
+  constexpr optional(const T&) noexcept;
+  template
+  constexpr optional(U&&) noexcept;
+  const T *() const;
+  T *();
+  const T () const;
+  T ();
+  T value_or(T) const;
+  };
+
+  template 
+  T&& move(T ) {
+return static_cast(x);
+  }
+}
+
+void takeOptionalValue(std::optional);
+void takeOptionalRef(const std::optional&);
+void takeOptionalRRef(std::optional&&);
+void takeOtherOptional(std::optional);
+
+void incorrect(std::optional param)
+{
+  std::optional* ptr = 
+  takeOptionalValue(**ptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalValue(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(param.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(param.operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalRef(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(param);
+  takeOptionalRef(param.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(param);
+  takeOptionalRef(param.operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(param);
+  std::optional p = *param;
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: std::optional p = param;
+
+  takeOptionalValue(std::move(**ptr));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(std::move(*ptr));
+  takeOptionalValue(std::move(*param));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(std::move(param));
+  takeOptionalValue(std::move(param.value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional 

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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D147357#4256636 , @PiotrZSL wrote:

> As for `takeOptionalValue(std::move(*param));`, it could be added as an 
> configuration option for PassthroughFunctions or UtilityFunctions. I can 
> thing about that, that should be easy to implement.

That's overcomplicating things, just look for `std::move` and you'll cover 
99.9% of instances in real world code.




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst:34
+Value extraction using ``operator *`` is matched by default.
+Check does not provide auto-fixes.
+

PiotrZSL wrote:
> njames93 wrote:
> > Any reason for this limitation, given in most cases the fix is to just 
> > remove the `*` or `.value()` call
> Sometimes problem is not with optional but with called function that take 
> optional when it could take value.
> At least for such issues I run, still probably could be safe to introduce 
> such auto-fix to just remove */.value().
> But then question would be if it should be bugprone or readability check.
> For me like 10% of these issues show some possible bug around that code.
That's a good point, if they are hiding a bug we don't want to auto fix it.
Perhaps the fix should be added as a note
```
note: Remove the call to '%FuncName%' to silence this warning
note: Remove '*' to silence this warning
```



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp:16
+  };
+}
+

PiotrZSL wrote:
> njames93 wrote:
> > It'd be good to have tests demonstating `boost::optional` and 
> > `absl::optional` as well as the (non standard) get accessor
> I added them to config just to add them, as they should work, I could add 
> some tests for them, but behavior should be same...
On the subject of boost(unsure about absl as I've never used that library)
Would you ever add support for `T& boost::get(boost::optional &)` or its 
const ref cousin?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp:37
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: conversion from 
'std::optional' into 'int' and back into 'std::optional', remove 
potentially error-prone optional dereference 
[bugprone-optional-value-conversion]
+}
+

What happens with this pathological case
`takeOptionalValue(param.operator*());`
Looks like it isn't being handled, should probably handle(and test) for it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

As for `takeOptionalValue(std::move(*param));`, it could be added as an 
configuration option for PassthroughFunctions or UtilityFunctions. I can thing 
about that, that should be easy to implement.




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst:34
+Value extraction using ``operator *`` is matched by default.
+Check does not provide auto-fixes.
+

njames93 wrote:
> Any reason for this limitation, given in most cases the fix is to just remove 
> the `*` or `.value()` call
Sometimes problem is not with optional but with called function that take 
optional when it could take value.
At least for such issues I run, still probably could be safe to introduce such 
auto-fix to just remove */.value().
But then question would be if it should be bugprone or readability check.
For me like 10% of these issues show some possible bug around that code.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp:16
+  };
+}
+

njames93 wrote:
> It'd be good to have tests demonstating `boost::optional` and 
> `absl::optional` as well as the (non standard) get accessor
I added them to config just to add them, as they should work, I could add some 
tests for them, but behavior should be same...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Have you thought of the possibility of handling this case

  takeOptionalValue(std::move(*param));




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst:34
+Value extraction using ``operator *`` is matched by default.
+Check does not provide auto-fixes.
+

Any reason for this limitation, given in most cases the fix is to just remove 
the `*` or `.value()` call



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp:16
+  };
+}
+

It'd be good to have tests demonstating `boost::optional` and `absl::optional` 
as well as the (non standard) get accessor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Piotr Zegar via Phabricator via cfe-commits]

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

Fix doc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-optional-value-conversion %t
+
+namespace std {
+  template
+  struct optional
+  {
+  constexpr optional() noexcept;
+  constexpr optional(T&&) noexcept;
+  constexpr optional(const T&) noexcept;
+  template
+  constexpr optional(U&&) noexcept;
+  const T *() const;
+  const T () const;
+  T value_or(T) const;
+  };
+}
+
+void takeOptionalValue(std::optional);
+void takeOptionalRef(const std::optional&);
+void takeOtherOptional(std::optional);
+
+void incorrect(std::optional param)
+{
+  takeOptionalValue(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+
+  takeOptionalValue(param.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+
+  takeOptionalRef(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+
+  takeOptionalRef(param.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  std::optional p = *param;
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+}
+
+void correct(std::optional param)
+{
+  takeOtherOptional(*param);
+
+  takeOtherOptional(param.value());
+
+  takeOtherOptional(param.value_or(5U));
+
+  std::optional p = *param;
+
+  takeOptionalValue(param.value_or(5U));
+
+  takeOptionalRef(param.value_or(5U));
+}
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
@@ -105,6 +105,7 @@
`bugprone-multiple-statement-macro `_,
`bugprone-no-escape `_,
`bugprone-not-null-terminated-result `_, "Yes"
+   `bugprone-optional-value-conversion `_,
`bugprone-parent-virtual-call `_, "Yes"
`bugprone-posix-return `_, "Yes"
`bugprone-redundant-branch-condition `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst
@@ -0,0 +1,49 @@
+.. title:: clang-tidy - bugprone-optional-value-conversion
+
+bugprone-optional-value-conversion
+==
+
+Detects potentially unintentional and redundant conversions where a value is
+extracted from an optional-like type and then used to create a new instance of
+the same optional-like type.
+
+These conversions might be the result of developer oversight, leftovers from
+code refactoring, or other situations that could lead to unintended exceptions
+or cases where the resulting optional is always initialized, which might be
+unexpected behavior.
+
+.. code-block:: c++
+
+#include 
+
+void print(std::optional);
+
+int main()
+{
+  std::optional opt;
+  // ...
+
+  // Unintentional conversion from std::optional to int and back to
+  // std::optional:
+  print(opt.value());
+
+  // ...
+}
+
+Value extraction using ``operator *`` is matched by default.
+Check does not provide auto-fixes.
+
+Options:
+
+
+.. option:: OptionalTypes
+
+Semicolon-separated list of (fully qualified) optional type names or 

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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

There are 37 findings in llvm repository:

1. clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:96:12: warning: conversion 
from 'std::optional' into 'bool' and back into 'std::optional', 
remove potentially error-prone optional dereference 
[bugprone-optional-value-conversion]
2. clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:65:12: warning: 
conversion from 'std::optional>' into 
'std::basic_string' and back into 
'std::optional>', remove potentially error-prone 
optional dereference [bugprone-optional-value-conversion]
3. clang/lib/AST/Interp/Program.cpp:173:12: warning: conversion from 
'std::optional' into 'unsigned int' and back into 
'std::optional', remove potentially error-prone optional 
dereference [bugprone-optional-value-conversion]
4. clang/lib/Analysis/PathDiagnostic.cpp:323:14: warning: conversion from 
'std::optional' into 'bool' and back into 'std::optional', remove 
potentially error-prone optional dereference 
[bugprone-optional-value-conversion]
5. clang/lib/Frontend/Rewrite/InclusionRewriter.cpp:253:7: warning: conversion 
from 'std::optional' into 'llvm::MemoryBufferRef' and 
back into 'std::optional', remove potentially 
error-prone optional dereference [bugprone-optional-value-conversion]
6. clang/lib/Sema/Scope.cpp:190:25: warning: conversion from 
'std::optional' into 'clang::VarDecl *' and back into 
'std::optional', remove potentially error-prone optional 
dereference [bugprone-optional-value-conversion]
7. clang/lib/Sema/TreeTransform.h:13915:9: warning: conversion from 
'std::optional' into 'unsigned int' and back into 
'std::optional', remove potentially error-prone optional 
dereference [bugprone-optional-value-conversion]
8. clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:440:16: warning: 
conversion from 'std::optional' into 'int' and back into 
'std::optional', remove potentially error-prone optional dereference 
[bugprone-optional-value-conversion]
9. clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp:125:10: 
warning: conversion from 'std::optional' into 'bool' and back into 
'std::optional', remove potentially error-prone optional dereference 
[bugprone-optional-value-conversion]
10. clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1573:10: warning: conversion 
from 'std::optional' into 
'clang::ento::nonloc::LazyCompoundVal' and back into 
'std::optional', remove potentially 
error-prone optional dereference [bugprone-optional-value-conversion]
11. clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1784:12: warning: conversion 
from 'std::optional' into 'clang::ento::SVal' and back into 
'std::optional', remove potentially error-prone optional 
dereference [bugprone-optional-value-conversion]
12. clang/lib/Tooling/InterpolatingCompilationDatabase.cpp:189:16: warning: 
conversion from 'std::optional' into 
'clang::driver::types::ID' and back into 
'std::optional', remove potentially error-prone 
optional dereference [bugprone-optional-value-conversion]
13. lldb/source/Breakpoint/BreakpointResolver.cpp:233:33: warning: conversion 
from 'std::optional' into 'unsigned short' and back into 
'std::optional', remove potentially error-prone optional 
dereference [bugprone-optional-value-conversion]
14. lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1362:53: 
warning: conversion from 'std::optional' into 'unsigned long' 
and back into 'std::optional', remove potentially error-prone 
optional dereference [bugprone-optional-value-conversion]
15. llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:4119:12: warning: 
conversion from 'std::optional' into 
'LiveDebugValues::ValueIDNum' and back into 
'std::optional', remove potentially error-prone 
optional dereference [bugprone-optional-value-conversion]
16. llvm/lib/DWARFLinker/DWARFLinker.cpp:1191:14: warning: conversion from 
'std::optional' into 'unsigned long' and back into 
'std::optional', remove potentially error-prone optional 
dereference [bugprone-optional-value-conversion]
17. llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:1305:31: warning: conversion from 
'std::optional' into 'long' and back into 'std::optional', remove 
potentially error-prone optional dereference 
[bugprone-optional-value-conversion]
18. llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:1322:62: warning: conversion from 
'std::optional' into 'llvm::DWARFFormValue' and back into 
'std::optional', remove potentially error-prone optional 
dereference [bugprone-optional-value-conversion]
19. llvm/lib/DebugInfo/Symbolize/Markup.cpp:67:14: warning: conversion from 
'std::optional' into 'llvm::symbolize::MarkupNode' 
and back into 'std::optional', remove potentially 
error-prone optional dereference [bugprone-optional-value-conversion]
20. llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp:425:13: warning: conversion 
from 'std::optional' into 
'llvm::raw_ostream::Colors' and back into 
'std::optional', remove potentially error-prone 
optional dereference [bugprone-optional-value-conversion]
21. 

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst:46
+expressions that match the methods.
+Default value is `"::value$;::get$`.

Unintended quote in value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

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

[Piotr Zegar via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147357

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

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-optional-value-conversion %t
+
+namespace std {
+  template
+  struct optional
+  {
+  constexpr optional() noexcept;
+  constexpr optional(T&&) noexcept;
+  constexpr optional(const T&) noexcept;
+  template
+  constexpr optional(U&&) noexcept;
+  const T *() const;
+  const T () const;
+  T value_or(T) const;
+  };
+}
+
+void takeOptionalValue(std::optional);
+void takeOptionalRef(const std::optional&);
+void takeOtherOptional(std::optional);
+
+void incorrect(std::optional param)
+{
+  takeOptionalValue(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+
+  takeOptionalValue(param.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+
+  takeOptionalRef(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+
+  takeOptionalRef(param.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  std::optional p = *param;
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: conversion from 'std::optional' into 'int' and back into 'std::optional', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+}
+
+void correct(std::optional param)
+{
+  takeOtherOptional(*param);
+
+  takeOtherOptional(param.value());
+
+  takeOtherOptional(param.value_or(5U));
+
+  std::optional p = *param;
+
+  takeOptionalValue(param.value_or(5U));
+
+  takeOptionalRef(param.value_or(5U));
+}
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
@@ -105,6 +105,7 @@
`bugprone-multiple-statement-macro `_,
`bugprone-no-escape `_,
`bugprone-not-null-terminated-result `_, "Yes"
+   `bugprone-optional-value-conversion `_,
`bugprone-parent-virtual-call `_, "Yes"
`bugprone-posix-return `_, "Yes"
`bugprone-redundant-branch-condition `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - bugprone-optional-value-conversion
+
+bugprone-optional-value-conversion
+==
+
+Detects potentially unintentional and redundant conversions where a value is
+extracted from an optional-like type and then used to create a new instance of
+the same optional-like type.
+
+These conversions might be the result of developer oversight, leftovers from
+code refactoring, or other situations that could lead to unintended exceptions
+or cases where the resulting optional is always initialized, which might be
+unexpected behavior.
+
+.. code-block:: c++
+
+#include 
+
+void print(std::optional);
+
+int main()
+{
+  std::optional opt;
+  // ...
+
+  // Unintentional conversion from std::optional to int and