[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG66945b62f42f: Add Optional overload to DiagnosticBuilder 
operator << (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75714

Files:
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang/include/clang/Basic/Diagnostic.h

Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -1288,6 +1289,29 @@
   return DB;
 }
 
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+}
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+}
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB, const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+}
+
 /// A nullability kind paired with a bit indicating whether it used a
 /// context-sensitive keyword.
 using DiagNullabilityKind = std::pair;
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -429,11 +429,9 @@
   if (MakeSmartPtrFunctionHeader.empty()) {
 return;
   }
-  if (auto IncludeFixit = Inserter->CreateIncludeInsertion(
-  FD, MakeSmartPtrFunctionHeader,
-  /*IsAngled=*/MakeSmartPtrFunctionHeader == StdMemoryHeader)) {
-Diag << *IncludeFixit;
-  }
+  Diag << Inserter->CreateIncludeInsertion(
+  FD, MakeSmartPtrFunctionHeader,
+  /*IsAngled=*/MakeSmartPtrFunctionHeader == StdMemoryHeader);
 }
 
 } // namespace modernize
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -86,13 +86,10 @@
   SourceRange(BaseRange.getEnd().getLocWithOffset(1),
   IndexRange.getBegin().getLocWithOffset(-1)),
   ", ")
-   << FixItHint::CreateReplacement(Matched->getEndLoc(), ")");
-
-  Optional Insertion = Inserter->CreateIncludeInsertion(
-  Result.SourceManager->getMainFileID(), GslHeader,
-  /*IsAngled=*/false);
-  if (Insertion)
-Diag << Insertion.getValue();
+   << FixItHint::CreateReplacement(Matched->getEndLoc(), ")")
+   << Inserter->CreateIncludeInsertion(
+  Result.SourceManager->getMainFileID(), GslHeader,
+  /*IsAngled=*/false);
 }
 return;
   }
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -96,10 +96,8 @@
MatchedDecl->getName().size()),
InitializationString);
 if (AddMathInclude) {
-  auto IncludeHint = IncludeInserter->CreateIncludeInsertion(
+  Diagnostic << IncludeInserter->CreateIncludeInsertion(
   Source.getFileID(MatchedDecl->getBeginLoc()), MathHeader, false);
-  if (IncludeHint)
-Diagnostic << *IncludeHint;
 }
   }
 }
Index: clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
@@ -105,12 +105,9 @@
 
   // Create a preprocessor #include FixIt hint (CreateIncludeInsertion checks
   // whether this already exists).
-  auto IncludeHint = IncludeInserter->CreateIncludeInsertion(
+  Diagnostic << IncludeInserter->CreateIncludeInsertion(
   Source.getFileID(ComparisonExpr->getBeginLoc()), AbseilStringsMatchHeader,
   false);
-  if (IncludeHint) {
-Diagnostic << *I

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 249361.
njames93 marked 2 inline comments as done.
njames93 added a comment.

- remove unnecessary semi colons


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75714

Files:
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang/include/clang/Basic/Diagnostic.h

Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -1288,6 +1289,29 @@
   return DB;
 }
 
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+}
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+}
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB, const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+}
+
 /// A nullability kind paired with a bit indicating whether it used a
 /// context-sensitive keyword.
 using DiagNullabilityKind = std::pair;
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -429,11 +429,9 @@
   if (MakeSmartPtrFunctionHeader.empty()) {
 return;
   }
-  if (auto IncludeFixit = Inserter->CreateIncludeInsertion(
-  FD, MakeSmartPtrFunctionHeader,
-  /*IsAngled=*/MakeSmartPtrFunctionHeader == StdMemoryHeader)) {
-Diag << *IncludeFixit;
-  }
+  Diag << Inserter->CreateIncludeInsertion(
+  FD, MakeSmartPtrFunctionHeader,
+  /*IsAngled=*/MakeSmartPtrFunctionHeader == StdMemoryHeader);
 }
 
 } // namespace modernize
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -86,13 +86,10 @@
   SourceRange(BaseRange.getEnd().getLocWithOffset(1),
   IndexRange.getBegin().getLocWithOffset(-1)),
   ", ")
-   << FixItHint::CreateReplacement(Matched->getEndLoc(), ")");
-
-  Optional Insertion = Inserter->CreateIncludeInsertion(
-  Result.SourceManager->getMainFileID(), GslHeader,
-  /*IsAngled=*/false);
-  if (Insertion)
-Diag << Insertion.getValue();
+   << FixItHint::CreateReplacement(Matched->getEndLoc(), ")")
+   << Inserter->CreateIncludeInsertion(
+  Result.SourceManager->getMainFileID(), GslHeader,
+  /*IsAngled=*/false);
 }
 return;
   }
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -96,10 +96,8 @@
MatchedDecl->getName().size()),
InitializationString);
 if (AddMathInclude) {
-  auto IncludeHint = IncludeInserter->CreateIncludeInsertion(
+  Diagnostic << IncludeInserter->CreateIncludeInsertion(
   Source.getFileID(MatchedDecl->getBeginLoc()), MathHeader, false);
-  if (IncludeHint)
-Diagnostic << *IncludeHint;
 }
   }
 }
Index: clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
@@ -105,12 +105,9 @@
 
   // Create a preprocessor #include FixIt hint (CreateIncludeInsertion checks
   // whether this already exists).
-  auto IncludeHint = IncludeInserter->CreateIncludeInsertion(
+  Diagnostic << IncludeInserter->CreateIncludeInsertion(
   Source.getFileID(ComparisonExpr->getBeginLoc()), AbseilStringsMatchHeader,
   false);
-  if (IncludeHint) {
-Diagnostic << *IncludeHint;
-  }
 }
 
 void StringFi

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Basic/Diagnostic.h:1298
+  return DB;
+};
+

s/;// (and in functions below as well)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75714



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

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 249307.
njames93 added a comment.
Herald added subscribers: arphaman, kbarton, nemanjai.

- Add usages, removed optional on arrayrefs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75714

Files:
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang/include/clang/Basic/Diagnostic.h

Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -1288,6 +1289,29 @@
   return DB;
 }
 
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB, const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
 /// A nullability kind paired with a bit indicating whether it used a
 /// context-sensitive keyword.
 using DiagNullabilityKind = std::pair;
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -429,11 +429,9 @@
   if (MakeSmartPtrFunctionHeader.empty()) {
 return;
   }
-  if (auto IncludeFixit = Inserter->CreateIncludeInsertion(
-  FD, MakeSmartPtrFunctionHeader,
-  /*IsAngled=*/MakeSmartPtrFunctionHeader == StdMemoryHeader)) {
-Diag << *IncludeFixit;
-  }
+  Diag << Inserter->CreateIncludeInsertion(
+  FD, MakeSmartPtrFunctionHeader,
+  /*IsAngled=*/MakeSmartPtrFunctionHeader == StdMemoryHeader);
 }
 
 } // namespace modernize
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -86,13 +86,10 @@
   SourceRange(BaseRange.getEnd().getLocWithOffset(1),
   IndexRange.getBegin().getLocWithOffset(-1)),
   ", ")
-   << FixItHint::CreateReplacement(Matched->getEndLoc(), ")");
-
-  Optional Insertion = Inserter->CreateIncludeInsertion(
-  Result.SourceManager->getMainFileID(), GslHeader,
-  /*IsAngled=*/false);
-  if (Insertion)
-Diag << Insertion.getValue();
+   << FixItHint::CreateReplacement(Matched->getEndLoc(), ")")
+   << Inserter->CreateIncludeInsertion(
+  Result.SourceManager->getMainFileID(), GslHeader,
+  /*IsAngled=*/false);
 }
 return;
   }
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -96,10 +96,8 @@
MatchedDecl->getName().size()),
InitializationString);
 if (AddMathInclude) {
-  auto IncludeHint = IncludeInserter->CreateIncludeInsertion(
+  Diagnostic << IncludeInserter->CreateIncludeInsertion(
   Source.getFileID(MatchedDecl->getBeginLoc()), MathHeader, false);
-  if (IncludeHint)
-Diagnostic << *IncludeHint;
 }
   }
 }
Index: clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
@@ -105,12 +105,9 @@
 
   // Create a preprocessor #include FixIt hint (CreateIncludeInsertion checks
   // whether this already exists).
-  auto IncludeHint = IncludeInserter->CreateIncludeInsertion(
+  Diagnostic << IncludeInserter->CreateIncludeInsertion(
   Source.getFileID(ComparisonExpr->getBeginLoc()), AbseilStringsMatchHeader,
   false);
-  if (IncludeHint) {
-Diagnostic << *IncludeHint

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D75714#1913230 , @aaron.ballman 
wrote:

> Is there any code we can cleanup as a result of adding these overloads? I 
> would have expected to see some code changes justifying each additional 
> overload, which would also give us test coverage for the changes.


I planned to add usages in follow ups but I can add them here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75714



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

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Is there any code we can cleanup as a result of adding these overloads? I would 
have expected to see some code changes justifying each additional overload, 
which would also give us test coverage for the changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75714



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

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 249121.
njames93 added a comment.

- Removed template in favour of specific overloads


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75714

Files:
  clang/include/clang/Basic/Diagnostic.h


Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -1288,6 +1289,45 @@
   return DB;
 }
 
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional> &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB, const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional> &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
 /// A nullability kind paired with a bit indicating whether it used a
 /// context-sensitive keyword.
 using DiagNullabilityKind = std::pair;


Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -1288,6 +1289,45 @@
   return DB;
 }
 
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional> &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB, const llvm::Optional &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional> &Opt) {
+  if (Opt)
+DB << *Opt;
+  return DB;
+};
+
 /// A nullability kind paired with a bit indicating whether it used a
 /// context-sensitive keyword.
 using DiagNullabilityKind = std::pair;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

I can see it being useful for fix-its and source ranges, however I have a 
concern about it accidentally eating arguments that were supposed to go into a 
format string. For that, I second Aaron's suggestion to implement an extension 
for format strings. If you don't want to do that far, I think adding specific 
overloads for optional fix-its and optional source ranges would work, and won't 
trigger in unexpected situations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75714



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

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D75714#1912400 , @njames93 wrote:

> In D75714#1912326 , @lebedev.ri 
> wrote:
>
> > What's the use case here?
>
>
> Mainly syntactic sugar, If you have a function that maybe returns a 
> `FixItHint` or `SourceRange` you can pass it directly to the 
> `DiagnosticBuilder`.


This comes up with a fair amount of regularity, even just for things like 
printing a simple integer (consider optional attribute arguments, etc). 
However, it seems like we might want to express this via the format string so 
that we don't run into a situation where an unavailable optional argument 
changes the indexing in the case where it's not additional information like a 
source range or fixit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75714



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

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D75714#1912326 , @lebedev.ri wrote:

> What's the use case here?


Mainly syntactic sugar, If you have a function that maybe returns a `FixItHint` 
or `SourceRange` you can pass it directly to the `DiagnosticBuilder`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75714



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

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

What's the use case here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75714



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

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Nathan James via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/Diagnostic.h:1318
+   const llvm::Optional &Opt) {
+  if (Opt) {
+DB << *Opt;

Should this be disabled on functions that pass format args to the diagnostic so 
it will only optionally add things FixIts, Notes, Ranges...?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75714



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

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75714

Files:
  clang/include/clang/Basic/Diagnostic.h


Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -1311,6 +1312,15 @@
   return DB;
 }
 
+template 
+inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional &Opt) {
+  if (Opt) {
+DB << *Opt;
+  }
+  return DB;
+}
+
 inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) {
   return Report(SourceLocation(), DiagID);
 }


Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -1311,6 +1312,15 @@
   return DB;
 }
 
+template 
+inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
+   const llvm::Optional &Opt) {
+  if (Opt) {
+DB << *Opt;
+  }
+  return DB;
+}
+
 inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) {
   return Report(SourceLocation(), DiagID);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits