Re: [PATCH] D22656: [Clang-apply-replacements] Remove custom version printing; fix some Include What You Use warnings

2016-07-23 Thread Alexander Kornienko via cfe-commits 
alexfh added a comment.

My last comment might come across as somewhat harsh. I didn't mean it to be 
harsh and just wanted to say that imo, llvm code is not ready to mechanically 
applying IWYU.


Repository:
  rL LLVM

https://reviews.llvm.org/D22656



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

Re: [PATCH] D22656: [Clang-apply-replacements] Remove custom version printing; fix some Include What You Use warnings

2016-07-23 Thread Alexander Kornienko via cfe-commits 
alexfh added a comment.

In https://reviews.llvm.org/D22656#493036, @Eugene.Zelenko wrote:

> Don't include DiagnosticIDs.h.


This header is just an example. I don't want anyone (including myself) manually 
figure out which of the headers inserted by IWYU actually need to be inserted. 
I'm also against adding #includes for every single transitively included 
header, since many of the headers can be considered "exported" by other 
headers. So, please, stop sending IWYU cleanups before reaching the general 
agreement and properly annotating LLVM and Clang headers.


Repository:
  rL LLVM

https://reviews.llvm.org/D22656



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

Re: [PATCH] D22656: [Clang-apply-replacements] Remove custom version printing; fix some Include What You Use warnings

2016-07-22 Thread Eugene Zelenko via cfe-commits 
Eugene.Zelenko updated this revision to Diff 65099.
Eugene.Zelenko added a comment.

Don't include DiagnosticIDs.h.


Repository:
  rL LLVM

https://reviews.llvm.org/D22656

Files:
  clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp

Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
===
--- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -16,13 +16,27 @@
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Rewrite/Core/RewriteBuffer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/ArrayRef.h"   // for make...
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 using namespace llvm;
 using namespace clang;
@@ -43,7 +57,6 @@
  "merging/replacing."),
 cl::init(false), cl::cat(ReplacementCategory));
 
-
 static cl::opt DoFormat(
 "format",
 cl::desc("Enable formatting of code changed by applying replacements.\n"
@@ -85,10 +98,6 @@
 };
 } // namespace
 
-static void printVersion() {
-  outs() << "clang-apply-replacements version " CLANG_VERSION_STRING << "\n";
-}
-
 /// \brief Convenience function to get rewritten content for \c Filename from
 /// \c Rewrites.
 ///
@@ -200,8 +209,6 @@
 
 int main(int argc, char **argv) {
   cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories));
-
-  cl::SetVersionPrinter();
   cl::ParseCommandLineOptions(argc, argv);
 
   IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions());


Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
===
--- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -16,13 +16,27 @@
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Rewrite/Core/RewriteBuffer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/ArrayRef.h"   // for make...
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 using namespace llvm;
 using namespace clang;
@@ -43,7 +57,6 @@
  "merging/replacing."),
 cl::init(false), cl::cat(ReplacementCategory));
 
-
 static cl::opt DoFormat(
 "format",
 cl::desc("Enable formatting of code changed by applying replacements.\n"
@@ -85,10 +98,6 @@
 };
 } // namespace
 
-static void printVersion() {
-  outs() << "clang-apply-replacements version " CLANG_VERSION_STRING << "\n";
-}
-
 /// \brief Convenience function to get rewritten content for \c Filename from
 /// \c Rewrites.
 ///
@@ -200,8 +209,6 @@
 
 int main(int argc, char **argv) {
   cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories));
-
-  cl::SetVersionPrinter();
   cl::ParseCommandLineOptions(argc, argv);
 
   IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22656: [Clang-apply-replacements] Remove custom version printing; fix some Include What You Use warnings

2016-07-22 Thread Alexander Kornienko via cfe-commits 
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

In many cases a transitively included header can be considered a part of a 
public interface of an already included header, e.g. it's reasonable to assume 
that DiagnosticIDs.h is a part of the public API exposed by Diagnostics.h. 
Thus, I consider IWYU fixes useless or even mildly harmful until (most of) 
LLVM/Clang headers are properly annotated with `// IWYU pragma: export` etc.

The other change seems fine, if you have properly verified the new behavior.


Repository:
  rL LLVM

https://reviews.llvm.org/D22656



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

Re: [PATCH] D22656: [Clang-apply-replacements] Remove custom version printing; fix some Include What You Use warnings

2016-07-21 Thread Eugene Zelenko via cfe-commits 
Eugene.Zelenko updated this revision to Diff 65004.
Eugene.Zelenko added a comment.

Full context diff.


Repository:
  rL LLVM

https://reviews.llvm.org/D22656

Files:
  clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp

Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
===
--- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -15,14 +15,29 @@
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Rewrite/Core/RewriteBuffer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/ArrayRef.h"   // for make...
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 using namespace llvm;
 using namespace clang;
@@ -43,7 +58,6 @@
  "merging/replacing."),
 cl::init(false), cl::cat(ReplacementCategory));
 
-
 static cl::opt DoFormat(
 "format",
 cl::desc("Enable formatting of code changed by applying replacements.\n"
@@ -85,10 +99,6 @@
 };
 } // namespace
 
-static void printVersion() {
-  outs() << "clang-apply-replacements version " CLANG_VERSION_STRING << "\n";
-}
-
 /// \brief Convenience function to get rewritten content for \c Filename from
 /// \c Rewrites.
 ///
@@ -200,8 +210,6 @@
 
 int main(int argc, char **argv) {
   cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories));
-
-  cl::SetVersionPrinter();
   cl::ParseCommandLineOptions(argc, argv);
 
   IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions());


Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
===
--- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -15,14 +15,29 @@
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Rewrite/Core/RewriteBuffer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/ArrayRef.h"   // for make...
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 using namespace llvm;
 using namespace clang;
@@ -43,7 +58,6 @@
  "merging/replacing."),
 cl::init(false), cl::cat(ReplacementCategory));
 
-
 static cl::opt DoFormat(
 "format",
 cl::desc("Enable formatting of code changed by applying replacements.\n"
@@ -85,10 +99,6 @@
 };
 } // namespace
 
-static void printVersion() {
-  outs() << "clang-apply-replacements version " CLANG_VERSION_STRING << "\n";
-}
-
 /// \brief Convenience function to get rewritten content for \c Filename from
 /// \c Rewrites.
 ///
@@ -200,8 +210,6 @@
 
 int main(int argc, char **argv) {
   cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories));
-
-  cl::SetVersionPrinter();
   cl::ParseCommandLineOptions(argc, argv);
 
   IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22656: [Clang-apply-replacements] Remove custom version printing; fix some Include What You Use warnings

2016-07-21 Thread Alexander Kornienko via cfe-commits 
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

Full context diffs, please. See 
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface


Repository:
  rL LLVM

https://reviews.llvm.org/D22656



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

[PATCH] D22656: [Clang-apply-replacements] Remove custom version printing; fix some Include What You Use warnings

2016-07-21 Thread Eugene Zelenko via cfe-commits 
Eugene.Zelenko created this revision.
Eugene.Zelenko added a reviewer: alexfh.
Eugene.Zelenko added a subscriber: cfe-commits.
Eugene.Zelenko set the repository for this revision to rL LLVM.

I checked this patch on my own build on RHEL 6. Regressions were OK.

Repository:
  rL LLVM

https://reviews.llvm.org/D22656

Files:
  clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp

Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
===
--- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -15,14 +15,29 @@
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Rewrite/Core/RewriteBuffer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/ArrayRef.h"   // for make...
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 using namespace llvm;
 using namespace clang;
@@ -43,7 +58,6 @@
  "merging/replacing."),
 cl::init(false), cl::cat(ReplacementCategory));
 
-
 static cl::opt DoFormat(
 "format",
 cl::desc("Enable formatting of code changed by applying replacements.\n"
@@ -85,10 +99,6 @@
 };
 } // namespace
 
-static void printVersion() {
-  outs() << "clang-apply-replacements version " CLANG_VERSION_STRING << "\n";
-}
-
 /// \brief Convenience function to get rewritten content for \c Filename from
 /// \c Rewrites.
 ///
@@ -200,8 +210,6 @@
 
 int main(int argc, char **argv) {
   cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories));
-
-  cl::SetVersionPrinter();
   cl::ParseCommandLineOptions(argc, argv);
 
   IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions());


Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
===
--- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -15,14 +15,29 @@
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Rewrite/Core/RewriteBuffer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/ArrayRef.h"   // for make...
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 using namespace llvm;
 using namespace clang;
@@ -43,7 +58,6 @@
  "merging/replacing."),
 cl::init(false), cl::cat(ReplacementCategory));
 
-
 static cl::opt DoFormat(
 "format",
 cl::desc("Enable formatting of code changed by applying replacements.\n"
@@ -85,10 +99,6 @@
 };
 } // namespace
 
-static void printVersion() {
-  outs() << "clang-apply-replacements version " CLANG_VERSION_STRING << "\n";
-}
-
 /// \brief Convenience function to get rewritten content for \c Filename from
 /// \c Rewrites.
 ///
@@ -200,8 +210,6 @@
 
 int main(int argc, char **argv) {
   cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories));
-
-  cl::SetVersionPrinter();
   cl::ParseCommandLineOptions(argc, argv);
 
   IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits