[PATCH] D113828: [clang-tidy] Fix false positives in `fuchsia-trailing-return` check involving deduction guides

[Julie Hockett via Phabricator via cfe-commits]

juliehockett resigned from this revision.
juliehockett added a comment.

Apologies, i's been a good long while since I was active in this area. @phosek 
is the most likely to know who would be the best contact in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113828

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

[PATCH] D66783: [clang-doc] Use llvm::createStringError and canonicalize error messages

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM -- thank you!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D66783



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

[PATCH] D66502: [clang-doc] Switch Generator::CreateResources to use llvm::Error

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG72e1f7f960d5: [clang-doc] Switch Generator::CreateResources 
to use llvm::Error (authored by juliehockett).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66502

Files:
  clang-tools-extra/clang-doc/Generators.cpp
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp

Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -326,8 +326,11 @@
 return 1;
 
   llvm::outs() << "Generating assets for docs...\n";
-  if (!G->get()->createResources(CDCtx))
+  Err = G->get()->createResources(CDCtx);
+  if (Err) {
+llvm::errs() << toString(std::move(Err)) << "\n";
 return 1;
+  }
 
   return 0;
 }
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -829,7 +829,7 @@
 
   llvm::Error generateDocForInfo(Info *I, llvm::raw_ostream ,
  const ClangDocContext ) override;
-  bool createResources(ClangDocContext ) override;
+  llvm::Error createResources(ClangDocContext ) override;
 };
 
 const char *HTMLGenerator::Format = "html";
@@ -883,7 +883,7 @@
   llvm_unreachable("Unknown InfoType");
 }
 
-static bool SerializeIndex(ClangDocContext ) {
+static llvm::Error SerializeIndex(ClangDocContext ) {
   std::error_code OK;
   std::error_code FileErr;
   llvm::SmallString<128> FilePath;
@@ -891,8 +891,9 @@
   llvm::sys::path::append(FilePath, "index_json.js");
   llvm::raw_fd_ostream OS(FilePath, FileErr, llvm::sys::fs::F_None);
   if (FileErr != OK) {
-llvm::errs() << "Error creating index file: " << FileErr.message() << "\n";
-return false;
+return llvm::make_error(
+"Error creating index file: " + FileErr.message() + "\n",
+llvm::inconvertibleErrorCode());
   }
   CDCtx.Idx.sort();
   llvm::json::OStream J(OS, 2);
@@ -911,7 +912,7 @@
   OS << "var JsonIndex = `\n";
   IndexToJSON(CDCtx.Idx);
   OS << "`;\n";
-  return true;
+  return llvm::Error::success();
 }
 
 // Generates a main HTML node that has the main content of the file that shows
@@ -932,15 +933,16 @@
   return MainNode;
 }
 
-static bool GenIndex(const ClangDocContext ) {
+static llvm::Error GenIndex(const ClangDocContext ) {
   std::error_code FileErr, OK;
   llvm::SmallString<128> IndexPath;
   llvm::sys::path::native(CDCtx.OutDirectory, IndexPath);
   llvm::sys::path::append(IndexPath, "index.html");
   llvm::raw_fd_ostream IndexOS(IndexPath, FileErr, llvm::sys::fs::F_None);
   if (FileErr != OK) {
-llvm::errs() << "Error creating main index: " << FileErr.message() << "\n";
-return false;
+return llvm::make_error(
+"Error creating main index: " + FileErr.message() + "\n",
+llvm::inconvertibleErrorCode());
   }
 
   HTMLFile F;
@@ -958,10 +960,10 @@
 
   F.Render(IndexOS);
 
-  return true;
+  return llvm::Error::success();
 }
 
-static bool CopyFile(StringRef FilePath, StringRef OutDirectory) {
+static llvm::Error CopyFile(StringRef FilePath, StringRef OutDirectory) {
   llvm::SmallString<128> PathWrite;
   llvm::sys::path::native(OutDirectory, PathWrite);
   llvm::sys::path::append(PathWrite, llvm::sys::path::filename(FilePath));
@@ -970,24 +972,33 @@
   std::error_code OK;
   std::error_code FileErr = llvm::sys::fs::copy_file(PathRead, PathWrite);
   if (FileErr != OK) {
-llvm::errs() << "Error creating file "
- << llvm::sys::path::filename(FilePath) << ": "
- << FileErr.message() << "\n";
-return false;
+return llvm::make_error(
+"Error creating file " + llvm::sys::path::filename(FilePath) + ": " +
+FileErr.message() + "\n",
+llvm::inconvertibleErrorCode());
   }
-  return true;
+  return llvm::Error::success();
 }
 
-bool HTMLGenerator::createResources(ClangDocContext ) {
-  if (!SerializeIndex(CDCtx) || !GenIndex(CDCtx))
-return false;
-  for (const auto  : CDCtx.UserStylesheets)
-if (!CopyFile(FilePath, CDCtx.OutDirectory))
-  return false;
-  for (const auto  : CDCtx.FilesToCopy)
-if (!CopyFile(FilePath, CDCtx.OutDirectory))
-  return false;
-  return true;
+llvm::Error HTMLGenerator::createResources(ClangDocContext ) {
+  auto Err = SerializeIndex(CDCtx);
+  if (Err)
+return Err;
+  Err = GenIndex(CDCtx);
+  if (Err)
+return Err;
+
+  for (const auto  : CDCtx.UserStylesheets) {
+Err = CopyFile(FilePath, CDCtx.OutDirectory);
+if (Err)
+  return Err;
+  }
+  for (const auto  : CDCtx.FilesToCopy) {
+Err = 

[PATCH] D66681: [clang-doc] Bump BitcodeWriter max line number to 32U

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369811: [clang-doc] Bump BitcodeWriter max line number to 
32U (authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66681?vs=216942=216952#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66681

Files:
  clang-tools-extra/trunk/clang-doc/BitcodeWriter.h


Index: clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
@@ -40,7 +40,7 @@
   static constexpr unsigned IntSize = 16U;
   static constexpr unsigned StringLengthSize = 16U;
   static constexpr unsigned FilenameLengthSize = 16U;
-  static constexpr unsigned LineNumberSize = 16U;
+  static constexpr unsigned LineNumberSize = 32U;
   static constexpr unsigned ReferenceTypeSize = 8U;
   static constexpr unsigned USRLengthSize = 6U;
   static constexpr unsigned USRBitLengthSize = 8U;


Index: clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
@@ -40,7 +40,7 @@
   static constexpr unsigned IntSize = 16U;
   static constexpr unsigned StringLengthSize = 16U;
   static constexpr unsigned FilenameLengthSize = 16U;
-  static constexpr unsigned LineNumberSize = 16U;
+  static constexpr unsigned LineNumberSize = 32U;
   static constexpr unsigned ReferenceTypeSize = 8U;
   static constexpr unsigned USRLengthSize = 6U;
   static constexpr unsigned USRBitLengthSize = 8U;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66681: [clang-doc] Bump BitcodeWriter max line number to 32U

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added a reviewer: phosek.
juliehockett added a project: clang-tools-extra.

b43039 reports hitting the assert on a very large file, so bumping this to 
allow for larger files.


https://reviews.llvm.org/D66681

Files:
  clang-tools-extra/clang-doc/BitcodeWriter.h


Index: clang-tools-extra/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/clang-doc/BitcodeWriter.h
+++ clang-tools-extra/clang-doc/BitcodeWriter.h
@@ -40,7 +40,7 @@
   static constexpr unsigned IntSize = 16U;
   static constexpr unsigned StringLengthSize = 16U;
   static constexpr unsigned FilenameLengthSize = 16U;
-  static constexpr unsigned LineNumberSize = 16U;
+  static constexpr unsigned LineNumberSize = 32U;
   static constexpr unsigned ReferenceTypeSize = 8U;
   static constexpr unsigned USRLengthSize = 6U;
   static constexpr unsigned USRBitLengthSize = 8U;


Index: clang-tools-extra/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/clang-doc/BitcodeWriter.h
+++ clang-tools-extra/clang-doc/BitcodeWriter.h
@@ -40,7 +40,7 @@
   static constexpr unsigned IntSize = 16U;
   static constexpr unsigned StringLengthSize = 16U;
   static constexpr unsigned FilenameLengthSize = 16U;
-  static constexpr unsigned LineNumberSize = 16U;
+  static constexpr unsigned LineNumberSize = 32U;
   static constexpr unsigned ReferenceTypeSize = 8U;
   static constexpr unsigned USRLengthSize = 6U;
   static constexpr unsigned USRBitLengthSize = 8U;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66502: [clang-doc] Switch Generator::CreateResources to use llvm::Error

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: DiegoAstiazaran, jakehehrlich.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman.

https://reviews.llvm.org/D66502

Files:
  clang-tools-extra/clang-doc/Generators.cpp
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp

Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -326,8 +326,11 @@
 return 1;
 
   llvm::outs() << "Generating assets for docs...\n";
-  if (!G->get()->createResources(CDCtx))
+  Err = G->get()->createResources(CDCtx);
+  if (Err) {
+llvm::errs() << toString(std::move(Err)) << "\n";
 return 1;
+  }
 
   return 0;
 }
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -829,7 +829,7 @@
 
   llvm::Error generateDocForInfo(Info *I, llvm::raw_ostream ,
  const ClangDocContext ) override;
-  bool createResources(ClangDocContext ) override;
+  llvm::Error createResources(ClangDocContext ) override;
 };
 
 const char *HTMLGenerator::Format = "html";
@@ -883,7 +883,7 @@
   llvm_unreachable("Unknown InfoType");
 }
 
-static bool SerializeIndex(ClangDocContext ) {
+static llvm::Error SerializeIndex(ClangDocContext ) {
   std::error_code OK;
   std::error_code FileErr;
   llvm::SmallString<128> FilePath;
@@ -891,8 +891,9 @@
   llvm::sys::path::append(FilePath, "index_json.js");
   llvm::raw_fd_ostream OS(FilePath, FileErr, llvm::sys::fs::F_None);
   if (FileErr != OK) {
-llvm::errs() << "Error creating index file: " << FileErr.message() << "\n";
-return false;
+return llvm::make_error(
+"Error creating index file: " + FileErr.message() + "\n",
+llvm::inconvertibleErrorCode());
   }
   CDCtx.Idx.sort();
   llvm::json::OStream J(OS, 2);
@@ -911,7 +912,7 @@
   OS << "var JsonIndex = `\n";
   IndexToJSON(CDCtx.Idx);
   OS << "`;\n";
-  return true;
+  return llvm::Error::success();
 }
 
 // Generates a main HTML node that has the main content of the file that shows
@@ -932,15 +933,16 @@
   return MainNode;
 }
 
-static bool GenIndex(const ClangDocContext ) {
+static llvm::Error GenIndex(const ClangDocContext ) {
   std::error_code FileErr, OK;
   llvm::SmallString<128> IndexPath;
   llvm::sys::path::native(CDCtx.OutDirectory, IndexPath);
   llvm::sys::path::append(IndexPath, "index.html");
   llvm::raw_fd_ostream IndexOS(IndexPath, FileErr, llvm::sys::fs::F_None);
   if (FileErr != OK) {
-llvm::errs() << "Error creating main index: " << FileErr.message() << "\n";
-return false;
+return llvm::make_error(
+"Error creating main index: " + FileErr.message() + "\n",
+llvm::inconvertibleErrorCode());
   }
 
   HTMLFile F;
@@ -958,10 +960,10 @@
 
   F.Render(IndexOS);
 
-  return true;
+  return llvm::Error::success();
 }
 
-static bool CopyFile(StringRef FilePath, StringRef OutDirectory) {
+static llvm::Error CopyFile(StringRef FilePath, StringRef OutDirectory) {
   llvm::SmallString<128> PathWrite;
   llvm::sys::path::native(OutDirectory, PathWrite);
   llvm::sys::path::append(PathWrite, llvm::sys::path::filename(FilePath));
@@ -970,24 +972,33 @@
   std::error_code OK;
   std::error_code FileErr = llvm::sys::fs::copy_file(PathRead, PathWrite);
   if (FileErr != OK) {
-llvm::errs() << "Error creating file "
- << llvm::sys::path::filename(FilePath) << ": "
- << FileErr.message() << "\n";
-return false;
+return llvm::make_error(
+"Error creating file " + llvm::sys::path::filename(FilePath) + ": " +
+FileErr.message() + "\n",
+llvm::inconvertibleErrorCode());
   }
-  return true;
+  return llvm::Error::success();
 }
 
-bool HTMLGenerator::createResources(ClangDocContext ) {
-  if (!SerializeIndex(CDCtx) || !GenIndex(CDCtx))
-return false;
-  for (const auto  : CDCtx.UserStylesheets)
-if (!CopyFile(FilePath, CDCtx.OutDirectory))
-  return false;
-  for (const auto  : CDCtx.FilesToCopy)
-if (!CopyFile(FilePath, CDCtx.OutDirectory))
-  return false;
-  return true;
+llvm::Error HTMLGenerator::createResources(ClangDocContext ) {
+  auto Err = SerializeIndex(CDCtx);
+  if (Err)
+return Err;
+  Err = GenIndex(CDCtx);
+  if (Err)
+return Err;
+
+  for (const auto  : CDCtx.UserStylesheets) {
+Err = CopyFile(FilePath, CDCtx.OutDirectory);
+if (Err)
+  return Err;
+  }
+  for (const auto  : CDCtx.FilesToCopy) {
+Err = CopyFile(FilePath, CDCtx.OutDirectory);
+if (Err)
+  return Err;
+  }
+  return llvm::Error::success();
 }
 
 static 

[PATCH] D66378: [clang-doc] Fix casting not working in gcc 5.4.0

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

LGTM, for posterity


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66378



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

[PATCH] D66353: [clang-doc] Redesign of generated HTML files

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D66353



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

[PATCH] D66353: [clang-doc] Redesign of generated HTML files

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:487-489
+static std::unique_ptr
+genFileMainNode(StringRef InfoPath,
+std::vector> ,

Would you be able to briefly comment on each of these `gen*Node` functions on 
what exactly they're doing? I'm having trouble getting a picture of what the 
flow is there



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:523
+  auto SpanNode =
+  std::make_unique(HTMLTag::TAG_SPAN, "clang-doc version 
unknown");
+  SpanNode->Attributes.emplace_back("class", "no-break");

Try using `getClangToolFullVersion` for here? 
https://clang.llvm.org/doxygen/namespaceclang.html#a35fdf9e05d2640414289e62cc837bf78



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:828
+  std::vector> MainContentNodes;
+  std::make_unique(HTMLTag::TAG_DIV);
   Index InfoIndex;

This isn't doing anything


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

https://reviews.llvm.org/D66353



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

[PATCH] D66298: [clang-doc] Fix records in global namespace

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:299
   RecordInfo *G = InfoAsRecord(Infos[2].get());
-  RecordInfo ExpectedG(EmptySID, /*Name=*/"G", /*Path=*/"E");
+  RecordInfo ExpectedG(EmptySID, /*Name=*/"G", /*Path=*/"GlobalNamespace/E");
   ExpectedG.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});

Native path?



Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:434
   ExpectedParentB.ChildRecords.emplace_back(EmptySID, "B", InfoType::IT_record,
-"A");
+"GlobalNamespace/A");
   CheckRecordInfo(, ParentB);

Native path?


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

https://reviews.llvm.org/D66298



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

[PATCH] D66238: [clang-doc] Serialize inherited attributes and methods

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/BitcodeWriter.h:33
 // BitCodeConstants, though they can be added without breaking it.
 static const unsigned VersionNumber = 2;
 

I definitely haven't been particularly good about bumping this, but can you 
bump it? This is a decently large change.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:233
 
-static void parseFields(RecordInfo , const RecordDecl *D, bool PublicOnly) {
+static bool documentInfo(bool PublicOnly, bool IsInAnonymousNamespace,
+ const NamedDecl *D) {

s/documentInfo/shouldSerializeInfo



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:277
   for (const FieldDecl *F : D->fields()) {
-if (PublicOnly && !isPublic(F->getAccessUnsafe(), F->getLinkageInternal()))
+if (!documentInfo(PublicOnly, false, F))
   continue;

s/false/`/*IsInAnonymousNamespace=*/false` 



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:436
+  if (const CXXRecordDecl *Base =
+  cast_or_null(Ty->getDecl()->getDefinition())) {
+bool IsVirtual = false;

Will `getDecl()` always return a non-null pointer? In the normal case I'd 
assume so, but the `cast_or_null` will only catch a null coming out of 
`getDefinition()` or the cast, not `getDecl`, so just want to check.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:437-439
+bool IsVirtual = false;
+if (B.isVirtual())
+  IsVirtual = true;

Is there a reason this isn't `bool IsVirtual = B.IsVirtual()`?



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:440-446
+SymbolID USR = getUSRForDecl(Base);
+std::string BaseName = Base->getNameAsString();
+if (const auto *Ty = B.getType()->getAs()) 
{
+  const TemplateDecl *D = Ty->getTemplateName().getAsTemplateDecl();
+  USR = getUSRForDecl(D);
+  BaseName = B.getType().getAsString();
+}

Do this in an if/else -- `getNameAsString()` is a decently expensive operation, 
so only do it when you need it.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:447-450
+I.Bases.emplace_back(
+USR, BaseName, getInfoRelativePath(Base), IsVirtual,
+getFinalAccessSpecifier(ParentAccess, B.getAccessSpecifier()),
+IsParent);

It would be better to create the BaseRecordInfo and then move it into the 
vector, since the constructor will copy all those strings.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:454
+  if (const auto *MD = dyn_cast(Decl)) {
+// Don't inherit private methods
+if (MD->getAccessUnsafe() == AccessSpecifier::AS_private ||

s/inherit/serialize



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:460
+FI.IsMethod = true;
+// The seventh arg in populateParentNamespaces is a boolean
+// passed by reference, its value is not relevant in here so

s/populateParentNamespaces/populateFunctionInfo ?



Comment at: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp:91
+  I.Bases.back().ChildFunctions.back().Name = "InheritedFunctionOne";
+  I.Bases.back().Members.emplace_back("int", "path/to/int", "X",
+  AccessSpecifier::AS_private);

For clarity, call this something else? Since it already has an `X` private 
member.


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

https://reviews.llvm.org/D66238



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

[PATCH] D66151: [clang-doc] Fix bitcode writer

[Julie Hockett via Phabricator via cfe-commits]

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

One comment then LGTM




Comment at: clang-tools-extra/clang-doc/Serialize.cpp:507
   Reference{ParentUSR, Parent->getNameAsString(), InfoType::IT_record};
-  Func.Access = D->getAccess();
+  Func.Access = D->getAccessUnsafe();
 

In a C++ method, it should have an access associated with it, and so AS_none 
isn't a valid value.


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

https://reviews.llvm.org/D66151



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

[PATCH] D65987: [clang-doc] Generate HTML links for children namespaces/records

[Julie Hockett via Phabricator via cfe-commits]

juliehockett accepted this revision.
juliehockett marked an inline comment as done.
juliehockett added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:250
+  // The resulting path is "../../../A/B/D" instead of a "../D". It is correct
+  // but it would be better to have the shorter version.
   StringRef Dir = Directory;

DiegoAstiazaran wrote:
> DiegoAstiazaran wrote:
> > juliehockett wrote:
> > > Would `llvm::sys::path::remove_dots()` do this? It might not, but is 
> > > worth investigating.
> > `llvm::sys::path::remove_dots()` removes all `../` (except for leading 
> > `../`) so the resulting path in the example would be `../A/B/D`, which is 
> > not correct.
> Sorry about that, I was incorrect. `llvm::sys::path::remove_dots()` do what 
> we want but from the right side of the path, not the left side.
> For `A/B/C/../..` it changes it to `A`. So it still does not work for us.
That's irritating :(


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

https://reviews.llvm.org/D65987



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

[PATCH] D65987: [clang-doc] Generate HTML links for children namespaces/records

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:250
+  // The resulting path is "../../../A/B/D" instead of a "../D". It is correct
+  // but it would be better to have the shorter version.
   StringRef Dir = Directory;

Would `llvm::sys::path::remove_dots()` do this? It might not, but is worth 
investigating.


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

https://reviews.llvm.org/D65987



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

[PATCH] D65483: [clang-doc] Add link to source code in file definitions

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D65483



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

[PATCH] D65915: [clang-doc] Protect Index with mutex during reducing and generation stage

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D65915



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

[PATCH] D65628: [clang-doc] Parallelize reducing phase

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM (after comment nit addressed)




Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:249
+  Error = false;
+  llvm::ThreadPool Pool(ExecutorConcurrency == 0 ? llvm::hardware_concurrency()
+ : ExecutorConcurrency);

nit: comment about where this var is coming from


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

https://reviews.llvm.org/D65628



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

[PATCH] D65483: [clang-doc] Add link to source code in file definitions

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/Representation.h:385
+SourceRoot(SourceRoot), UserStylesheets(UserStylesheets),
+JsScripts(JsScripts) {
+if (!RepositoryUrl.empty()) {

Move to .cpp file (now that it has a non-trivial implementation)



Comment at: clang-tools-extra/clang-doc/Representation.h:396
+  std::string OutDirectory; // Directory for outputting generated files.
+  std::string SourceRoot;   //
+  llvm::Optional RepositoryUrl;

Comment


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

https://reviews.llvm.org/D65483



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

[PATCH] D65030: [clang-doc] Add second index for sections within info's content

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

LGTM


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

https://reviews.llvm.org/D65030



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

[PATCH] D65827: [clang-doc] Fix paths of js in import tags

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM (let's make sure we double check this next time)


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

https://reviews.llvm.org/D65827



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

[PATCH] D65690: [clang-doc] Add index in each info html file

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D65690



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

[PATCH] D65622: [clang-doc] Update documentation

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

Sorry for the delayed review, but a few things I caught :)

Also, please make sure to update the the release notes/flag documentation for 
the patches you have in progress that add features or flags.




Comment at: clang-tools-extra/trunk/docs/ReleaseNotes.rst:55
 
-The improvements are...
+- :doc:`clang-doc ` now generates documentation in HTML format.
 

This should also include new flags and new features like template handling.



Comment at: clang-tools-extra/trunk/docs/clang-doc.rst:40
+
+  $ clang-doc /path/to/file.cpp -p /path/to/build
+

For this to work, you'd also need to pass the `--executor=standalone` flag.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65622



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

[PATCH] D65690: [clang-doc] Add index in each info html file

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/Generators.cpp:79
+
+bool Generator::createResources(ClangDocContext ) {
+  std::error_code OK;

Why is this implementation in the generic Generator? It's fairly HTML-specific 
-- neither of the other generators are able to parse and include Javascript 
(since this does include the `var JsonIndex` bit).



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:665
+return false;
+  llvm::outs() << "Generating assets for docs...\n";
+  for (const auto  : CDCtx.UserStylesheets)

Can we move this to `ClangDocMain.cpp` right above where the function is 
called? That's where most of these types of status updates are.


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

https://reviews.llvm.org/D65690



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

[PATCH] D65030: [clang-doc] Add second index for sections within info's content

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:47-52
+
+  Namespaces
+  Records
+  Functions
+  OneFunction
+

DiegoAstiazaran wrote:
> juliehockett wrote:
> > Formatting is a bit weird for sublists
> Fixed by D65005. It will be properly formatted after rebasing.
This formatting is still a bit weird :( The inner `ul` didn't get its own 
newline, so it's hard to tell which list each item is part of. Not a huge 
problem, but if it's a reasonably small fix I'd love to see it cleaned up.


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

https://reviews.llvm.org/D65030



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

[PATCH] D64958: [clang-doc] Fix link generation

[Julie Hockett via Phabricator via cfe-commits]

juliehockett accepted this revision.
juliehockett added a comment.

LGTM with a comment correction




Comment at: clang-tools-extra/clang-doc/Representation.h:117
   Reference(llvm::StringRef Name) : Name(Name) {}
-  Reference(llvm::StringRef Name, StringRef Path) : Name(Name), Path(Path) {}
+  // An empty path means the info is in the globalnamespace because the path is
+  // a composite of the parent namespaces.

s/globalnamespace/global namespace



Comment at: clang-tools-extra/clang-doc/Representation.h:123
   : USR(USR), Name(Name), RefType(IT) {}
+  // An empty path means the info is in the globalnamespace because the path is
+  // a composite of the parent namespaces.

s/globalnamespace/global namespace


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

https://reviews.llvm.org/D64958



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

[PATCH] D65483: [clang-doc] Add link to source code in file definitions

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:377-378
+  "href", (FileURL + "#" + std::to_string(L.LineNumber)).str());
+  Node->Children.emplace_back(std::move(LocNumberNode));
+  Node->Children.emplace_back(llvm::make_unique(" of file "));
+  auto LocFileNode = llvm::make_unique(

DiegoAstiazaran wrote:
> jakehehrlich wrote:
> > Can you put these in the link so that the link is larger than a single 
> > number? e.g. "303 of file foo.c" should be linkified rather than just "303" 
> > which is how I read the code now.
> "303" is linkified to the specific line in the source code but also "foo.c" 
> is linkified to the top of the source code page.
> This is how it is done in Doxygen documentation.
> I think it would look weird to have " of file " linkified. But talking to 
> @phosek, we agreed that it would be better to remove the whole "Defined at 
> ..." and make the info name linkified to the definition, with a tooltip that 
> shows "foo.c:303".
> So I think we could leave it like this for now and later, in another patch 
> (this will require some CSS), do what I just mentioned. What do you think?
While the tooltip idea seems interesting, I'm inclined to keep the "Defined at" 
portion and an actual link with the text for now. I'd want to see an example of 
what the tooltip option would look like before going with it. 

For now, the separate line number and file linkification SGTM.



Comment at: clang-tools-extra/clang-doc/Mapper.cpp:103-104
+  llvm::SmallString<128> Prefix(RootDir);
+  if (!llvm::sys::path::is_separator(Prefix.back()))
+Prefix += llvm::sys::path::get_separator();
+  llvm::sys::path::replace_path_prefix(File, Prefix, "");

DiegoAstiazaran wrote:
> jakehehrlich wrote:
> > Do you actually need to do this? Feels like a bug in replace_path_prefix 
> > per its own documentation if you do.
> `replace_path_prefix` simply calls substr() on the path so if the prefix does 
> not include the separator at the end, the resulting path will have it at the 
> beginning.
Leave a comment to that effect, then.



Comment at: clang-tools-extra/clang-doc/Mapper.cpp:40
+  bool IsFileInRootDir;
+  auto File = getFile(D, D->getASTContext(), CDCtx.RootDir, IsFileInRootDir);
+  auto I = serialize::emitInfo(D, getComment(D, D->getASTContext()),

Don't use auto here, since it's not immediately obvious what the type is.



Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:228
+  RepositoryUrl.find("https://;) != 0)
+RepositoryUrl.insert(0, "http://;);
+

DiegoAstiazaran wrote:
> jakehehrlich wrote:
> > 1) Do we need to add this for the user?
> > 2) Can we use https by default if we need this?
> Not required but I consider it'd be nice to have it.
> You're right, https should be the default.
This is fine, but please add a test case for the when the user omits the prefix.



Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:76
+static llvm::cl::opt
+RootDirectory("root-directory", llvm::cl::desc(R"(
+Directory where processed files are stored.

Can we call this `--source-root`?



Comment at: clang-tools-extra/docs/clang-doc.rst:93
+--public- Document only public declarations.
+--repository=   -
+  URL of repository that hosts code.

Formatting here is a bit weird


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

https://reviews.llvm.org/D65483



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

[PATCH] D65628: [clang-doc] Parallelize reducing phase

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:72
 
+static llvm::cl::opt ThreadCount(
+"thread-count",

Can we use the pre-existing concurrency flag instead 
(https://github.com/llvm/llvm-project/blob/91e5cdfc93729c61c757db4efd4a82670ac7f929/clang/lib/Tooling/AllTUsExecution.cpp#L150)?
 It seems counterintuitive to have to set two different concurrency flags for 
one tool.

You'll have to put up a separate patch to expose that flag in the AllTUs 
header, so that you can access it (see rL344335, as an example). 


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

https://reviews.llvm.org/D65628



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

[PATCH] D65309: [clang-format] Fix style of css file paths

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM after adding a comment to explain




Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:234
 llvm::sys::path::filename(FilePath));
+llvm::sys::path::native(StylesheetPath, llvm::sys::path::Style::posix);
 LinkNode->Attributes.try_emplace("href", StylesheetPath);

Comment here why you're explicitly using posix (i.e. HTML uses posix-style 
paths)


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

https://reviews.llvm.org/D65309



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

[PATCH] D65306: [clang-doc] Fix failing tests on Windows

[Julie Hockett via Phabricator via cfe-commits]

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

Make sure you update the other stylesheet patch, as well, before landing that.


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

https://reviews.llvm.org/D65306



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

[PATCH] D64938: [clang-doc] Add option for user provided stylesheets

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D64938



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

[PATCH] D65030: [clang-doc] Add second index for sections within info's content

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

Looks mostly good to me, with a small formatting thing




Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:490
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H3, I.Name));
+  Out.back()->Attributes.try_emplace("id",
+ llvm::toHex(llvm::toStringRef(I.USR)));

Comment here that you're using USR instead of name to disambiguate function 
overloads (mostly because I asked the question and had to think about why for a 
second, so might as well document it)



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:47-52
+
+  Namespaces
+  Records
+  Functions
+  OneFunction
+

Formatting is a bit weird for sublists


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

https://reviews.llvm.org/D65030



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

[PATCH] D65003: [clang-doc] Add index in each info html file

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/Generators.h:28
 
+  static Index genIndex(const std::vector> );
+

Add a comment here indicating that this should be created before calling any 
`generateDocForInfo`s. Also add a fix-it note to the following effect:

`FIXME: This currently needs to be run before generating any individual 
documentation pages, since the content it generates is directly included in 
every page. A better design would be to lazily include it in the individual 
documentation pages, in which case this could be run in parallel with calls to 
generateDocForInfo().`



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:337-346
+
+  A
+  B
+  C
+
+  D
+  E

The indentation here seems a bit off


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

https://reviews.llvm.org/D65003



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

[PATCH] D64958: [clang-doc] Fix link generation

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

Under what circumstances, exactly, is the path not set where you would need to 
create a link despite that? Is it only in the global namespace case? If so, 
could you special-case that and ignore other empty paths?




Comment at: clang-tools-extra/clang-doc/Representation.h:137
+// saved (possibly unresolved)
+  bool IsPathValid = false; // Indicates if a value has been assigned to Path,
+// it could be an empty string

Is this field actually set anywhere? 


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

https://reviews.llvm.org/D64958



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

[PATCH] D65107: [clang-doc] Fix html entities in rendered text

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D65107



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

[PATCH] D64938: [clang-doc] Add option for user provided stylesheets

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:26
+  ClangDocContext CDCtx;
+  CDCtx.UserStylesheets = {"../share/clang/clang-doc-default-stylesheet.css"};
+  return CDCtx;

Can you write a test for the output of more than one stylesheet?


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

https://reviews.llvm.org/D64938



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

[PATCH] D64539: [clang-doc] Add stylesheet to generated html docs

[Julie Hockett via Phabricator via cfe-commits]

juliehockett accepted this revision.
juliehockett added a comment.

LGTM


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

https://reviews.llvm.org/D64539



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

[PATCH] D64539: [clang-doc] Add stylesheet to generated html docs

[Julie Hockett via Phabricator via cfe-commits]

juliehockett accepted this revision.
juliehockett added a comment.

LGTM


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

https://reviews.llvm.org/D64539



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

[PATCH] D64813: [clang-tidy] Exclude forward decls from fuchsia-multiple-inheritance

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366354: [clang-tidy] Exclude forward decls from 
fuchsia-multiple-inheritance (authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64813?vs=210144=210367#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64813

Files:
  clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp


Index: clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -93,7 +93,8 @@
 return;
 
   // Match declarations which have bases.
-  Finder->addMatcher(cxxRecordDecl(hasBases()).bind("decl"), this);
+  Finder->addMatcher(
+  cxxRecordDecl(allOf(hasBases(), isDefinition())).bind("decl"), this);
 }
 
 void MultipleInheritanceCheck::check(const MatchFinder::MatchResult ) {
Index: clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -41,6 +41,9 @@
   virtual int baz() = 0;
 };
 
+// Shouldn't warn on forward declarations.
+class Bad_Child1;
+
 // Inherits from multiple concrete classes.
 // CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes that 
aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
 // CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};


Index: clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -93,7 +93,8 @@
 return;
 
   // Match declarations which have bases.
-  Finder->addMatcher(cxxRecordDecl(hasBases()).bind("decl"), this);
+  Finder->addMatcher(
+  cxxRecordDecl(allOf(hasBases(), isDefinition())).bind("decl"), this);
 }
 
 void MultipleInheritanceCheck::check(const MatchFinder::MatchResult ) {
Index: clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -41,6 +41,9 @@
   virtual int baz() = 0;
 };
 
+// Shouldn't warn on forward declarations.
+class Bad_Child1;
+
 // Inherits from multiple concrete classes.
 // CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes that aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
 // CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64669: [clang-doc] Fix failing tests on Windows

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365967: [clang-doc] Fix failing tests on Windows (authored 
by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64669?vs=209601=209617#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64669

Files:
  clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp


Index: clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -79,10 +79,11 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
+  SmallString<16> PathTo;
+  llvm::sys::path::native("path/to", PathTo);
   I.Members.emplace_back("int", "X/Y", "X", AccessSpecifier::AS_private);
   I.TagType = TagTypeKind::TTK_Class;
-  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record,
- llvm::SmallString<128>("path/to"));
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, PathTo);
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
 
   I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
@@ -97,6 +98,10 @@
   llvm::raw_string_ostream Actual(Buffer);
   auto Err = G->generateDocForInfo(, Actual);
   assert(!Err);
+  SmallString<16> PathToF;
+  llvm::sys::path::native("../../../path/to/F.html", PathToF);
+  SmallString<16> PathToInt;
+  llvm::sys::path::native("../int.html", PathToInt);
   std::string Expected = R"raw(
 
 class r
@@ -107,12 +112,14 @@
   
   
 Inherits from 
-F
+F
 , G
   
   Members
   
-private int X
+private int X
   
   Records
   
@@ -143,8 +150,10 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.ReturnType = TypeInfo(EmptySID, "float", InfoType::IT_default, "path/to");
-  I.Params.emplace_back("int", "path/to", "P");
+  SmallString<16> PathTo;
+  llvm::sys::path::native("path/to", PathTo);
+  I.ReturnType = TypeInfo(EmptySID, "float", InfoType::IT_default, PathTo);
+  I.Params.emplace_back("int", PathTo, "P");
   I.IsMethod = true;
   I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
 
@@ -154,15 +163,21 @@
   llvm::raw_string_ostream Actual(Buffer);
   auto Err = G->generateDocForInfo(, Actual);
   assert(!Err);
+  SmallString<16> PathToFloat;
+  llvm::sys::path::native("path/to/float.html", PathToFloat);
+  SmallString<16> PathToInt;
+  llvm::sys::path::native("path/to/int.html", PathToInt);
   std::string Expected = R"raw(
 
 
 
   f
   
-float
+float
  f(
-int
+int
  P)
   
   


Index: clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -79,10 +79,11 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
+  SmallString<16> PathTo;
+  llvm::sys::path::native("path/to", PathTo);
   I.Members.emplace_back("int", "X/Y", "X", AccessSpecifier::AS_private);
   I.TagType = TagTypeKind::TTK_Class;
-  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record,
- llvm::SmallString<128>("path/to"));
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, PathTo);
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
 
   I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
@@ -97,6 +98,10 @@
   llvm::raw_string_ostream Actual(Buffer);
   auto Err = G->generateDocForInfo(, Actual);
   assert(!Err);
+  SmallString<16> PathToF;
+  llvm::sys::path::native("../../../path/to/F.html", PathToF);
+  SmallString<16> PathToInt;
+  llvm::sys::path::native("../int.html", PathToInt);
   std::string Expected = R"raw(
 
 class r
@@ -107,12 +112,14 @@
   
   
 Inherits from 
-F
+F
 , G
   
   Members
   
-private int X
+private int X
   
   Records
   
@@ -143,8 +150,10 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.ReturnType = TypeInfo(EmptySID, "float", InfoType::IT_default, "path/to");
-  I.Params.emplace_back("int", "path/to", "P");
+  SmallString<16> PathTo;
+  llvm::sys::path::native("path/to", PathTo);
+  I.ReturnType = TypeInfo(EmptySID, "float", InfoType::IT_default, PathTo);
+  I.Params.emplace_back("int", PathTo, "P");
   I.IsMethod = true;
   I.Parent = Reference(EmptySID, "Parent", 

[PATCH] D64669: [clang-doc] Fix failing tests on Windows

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D64669



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

[PATCH] D63663: [clang-doc] Add html links to references

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365937: [clang-doc] Add html links to references (authored 
by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63663?vs=209534=209547#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63663

Files:
  clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
  clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
  clang-tools-extra/trunk/clang-doc/Generators.cpp
  clang-tools-extra/trunk/clang-doc/Generators.h
  clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/trunk/clang-doc/MDGenerator.cpp
  clang-tools-extra/trunk/clang-doc/Representation.cpp
  clang-tools-extra/trunk/clang-doc/Representation.h
  clang-tools-extra/trunk/clang-doc/Serialize.cpp
  clang-tools-extra/trunk/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -57,7 +57,7 @@
   
 OneFunction
 
-   OneFunction()
+  OneFunction()
 
   
   Enums
@@ -73,14 +73,16 @@
 TEST(HTMLGeneratorTest, emitRecordHTML) {
   RecordInfo I;
   I.Name = "r";
+  I.Path = "X/Y/Z";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.Members.emplace_back("int", "X", AccessSpecifier::AS_private);
+  I.Members.emplace_back("int", "X/Y", "X", AccessSpecifier::AS_private);
   I.TagType = TagTypeKind::TTK_Class;
-  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record);
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record,
+ llvm::SmallString<128>("path/to"));
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
 
   I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
@@ -104,11 +106,13 @@
 Defined at line 10 of test.cpp
   
   
-Inherits from F, G
+Inherits from 
+F
+, G
   
   Members
   
-private int X
+private int X
   
   Records
   
@@ -118,7 +122,7 @@
   
 OneFunction
 
-   OneFunction()
+  OneFunction()
 
   
   Enums
@@ -139,8 +143,8 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
-  I.Params.emplace_back("int", "P");
+  I.ReturnType = TypeInfo(EmptySID, "float", InfoType::IT_default, "path/to");
+  I.Params.emplace_back("int", "path/to", "P");
   I.IsMethod = true;
   I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
 
@@ -156,7 +160,10 @@
 
   f
   
-void f(int P)
+float
+ f(
+int
+ P)
   
   
 Defined at line 10 of test.cpp
@@ -261,8 +268,7 @@
  Brief description.
   
   
- Extended description that
- continues onto the next line.
+ Extended description that continues onto the next line.
   
 
   
Index: clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -25,6 +25,7 @@
 TEST(YAMLGeneratorTest, emitNamespaceYAML) {
   NamespaceInfo I;
   I.Name = "Namespace";
+  I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
@@ -45,6 +46,7 @@
   R"raw(---
 USR: ''
 Name:'Namespace'
+Path:'path/to/A'
 Namespace:
   - Type:Namespace
 Name:'A'
@@ -69,15 +71,18 @@
 TEST(YAMLGeneratorTest, emitRecordYAML) {
   RecordInfo I;
   I.Name = "r";
+  I.Path = "path/to/r";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.Members.emplace_back("int", "X", AccessSpecifier::AS_private);
+  I.Members.emplace_back("int", "path/to/int", "X",
+ AccessSpecifier::AS_private);
   I.TagType = TagTypeKind::TTK_Class;
-  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record);
-  

[PATCH] D64539: [clang-doc] Add stylesheet to generated html docs

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:242
 
+  // Generate css stylesheet
+  if (Format == "html") {

On second thought, can you create a virtual method to the Generator along the 
lines of `Generator::createResources(ClangDocContext ctx)`, which would be a 
no-op for the YAML and MD generators atm and for the HTML generator would 
encapsulate this logic? I'd rather not have it in the main tool file, since it 
is generator-specific.


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

https://reviews.llvm.org/D64539



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

[PATCH] D63663: [clang-doc] Add html links to references

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:110-113
+Inherits from 
+F
+, 
+G

This text should be rendered inline



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:168-171
+f(
+int
+ P
+)

This text should be rendered inline, if possible.


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

https://reviews.llvm.org/D63663



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

[PATCH] D64539: [clang-doc] Add stylesheet to generated html docs

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:247
+auto StylesheetPathWrite = getInfoOutputFile(
+OutDirectory, "", "clang-doc-default-stylesheet", ".css");
+if (!StylesheetPathWrite) {

Eventually, we'll want to make this a configurable thing via a flag, but that 
can be done in another patch.


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

https://reviews.llvm.org/D64539



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

[PATCH] D64539: [clang-doc] Add stylesheet to generated html docs

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:246-271
+auto StylesheetPathWrite = getInfoOutputFile(
+OutDirectory, "", "clang-doc-default-stylesheet", ".css");
+if (!StylesheetPathWrite) {
+  llvm::errs() << toString(StylesheetPathWrite.takeError()) << "\n";
+  return 1;
+}
+std::error_code FileErr;

Could you use  `llvm::sys::fs::copy_file` here? 
https://llvm.org/doxygen/namespacellvm_1_1sys_1_1fs.html#abe768b38d21bfc2bc91a1c1d09cd84de


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

https://reviews.llvm.org/D64539



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

[PATCH] D63857: [clang-doc] Add a structured HTML generator

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365687: [clang-doc] Add a structured HTML generator 
(authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63857?vs=208792=209040#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63857

Files:
  clang-tools-extra/trunk/clang-doc/CMakeLists.txt
  clang-tools-extra/trunk/clang-doc/Generators.cpp
  clang-tools-extra/trunk/clang-doc/Generators.h
  clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/trunk/clang-doc/MDGenerator.cpp
  clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/trunk/unittests/clang-doc/CMakeLists.txt
  clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp

Index: clang-tools-extra/trunk/unittests/clang-doc/CMakeLists.txt
===
--- clang-tools-extra/trunk/unittests/clang-doc/CMakeLists.txt
+++ clang-tools-extra/trunk/unittests/clang-doc/CMakeLists.txt
@@ -12,6 +12,7 @@
 add_extra_unittest(ClangDocTests
   BitcodeTest.cpp
   ClangDocTest.cpp
+  HTMLGeneratorTest.cpp
   MDGeneratorTest.cpp
   MergeTest.cpp
   SerializeTest.cpp
Index: clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -0,0 +1,276 @@
+//===-- clang-doc/HTMLGeneratorTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ClangDocTest.h"
+#include "Generators.h"
+#include "Representation.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace doc {
+
+std::unique_ptr getHTMLGenerator() {
+  auto G = doc::findGeneratorByName("html");
+  if (!G)
+return nullptr;
+  return std::move(G.get());
+}
+
+TEST(HTMLGeneratorTest, emitNamespaceHTML) {
+  NamespaceInfo I;
+  I.Name = "Namespace";
+  I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
+
+  I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
+ InfoType::IT_namespace);
+  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+  I.ChildFunctions.emplace_back();
+  I.ChildFunctions.back().Name = "OneFunction";
+  I.ChildEnums.emplace_back();
+  I.ChildEnums.back().Name = "OneEnum";
+
+  auto G = getHTMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(, Actual);
+  assert(!Err);
+  std::string Expected = R"raw(
+
+namespace Namespace
+
+  namespace Namespace
+  Namespaces
+  
+ChildNamespace
+  
+  Records
+  
+ChildStruct
+  
+  Functions
+  
+OneFunction
+
+   OneFunction()
+
+  
+  Enums
+  
+enum OneEnum
+  
+
+)raw";
+
+  EXPECT_EQ(Expected, Actual.str());
+}
+
+TEST(HTMLGeneratorTest, emitRecordHTML) {
+  RecordInfo I;
+  I.Name = "r";
+  I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
+
+  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
+
+  I.Members.emplace_back("int", "X", AccessSpecifier::AS_private);
+  I.TagType = TagTypeKind::TTK_Class;
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record);
+  I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
+
+  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+  I.ChildFunctions.emplace_back();
+  I.ChildFunctions.back().Name = "OneFunction";
+  I.ChildEnums.emplace_back();
+  I.ChildEnums.back().Name = "OneEnum";
+
+  auto G = getHTMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(, Actual);
+  assert(!Err);
+  std::string Expected = R"raw(
+
+class r
+
+  class r
+  
+Defined at line 10 of test.cpp
+  
+  
+Inherits from F, G
+  
+  Members
+  
+private int X
+  
+  Records
+  
+ChildStruct
+  
+  Functions
+  
+OneFunction
+
+   OneFunction()
+
+  
+  Enums
+  
+enum OneEnum
+  
+
+)raw";
+
+  EXPECT_EQ(Expected, Actual.str());
+}
+
+TEST(HTMLGeneratorTest, emitFunctionHTML) {
+  FunctionInfo I;
+  I.Name = "f";
+  I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
+
+  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
+
+  I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
+  

[PATCH] D64381: Update libc++ include path detection to use VFS on Linux

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365682: Update libc++ include path detection to use VFS on 
Linux (authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64381?vs=208546=209036#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64381

Files:
  cfe/trunk/lib/Driver/ToolChains/Linux.cpp


Index: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp
@@ -865,12 +865,13 @@
   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
 }
 
-static std::string DetectLibcxxIncludePath(StringRef base) {
+static std::string DetectLibcxxIncludePath(llvm::vfs::FileSystem ,
+   StringRef base) {
   std::error_code EC;
   int MaxVersion = 0;
   std::string MaxVersionString = "";
-  for (llvm::sys::fs::directory_iterator LI(base, EC), LE; !EC && LI != LE;
-   LI = LI.increment(EC)) {
+  for (llvm::vfs::directory_iterator LI = vfs.dir_begin(base, EC), LE;
+   !EC && LI != LE; LI = LI.increment(EC)) {
 StringRef VersionText = llvm::sys::path::filename(LI->path());
 int Version;
 if (VersionText[0] == 'v' &&
@@ -888,12 +889,12 @@
   llvm::opt::ArgStringList ) const {
   const std::string& SysRoot = computeSysRoot();
   const std::string LibCXXIncludePathCandidates[] = {
-  DetectLibcxxIncludePath(getDriver().Dir + "/../include/c++"),
+  DetectLibcxxIncludePath(getVFS(), getDriver().Dir + "/../include/c++"),
   // If this is a development, non-installed, clang, libcxx will
   // not be found at ../include/c++ but it likely to be found at
   // one of the following two locations:
-  DetectLibcxxIncludePath(SysRoot + "/usr/local/include/c++"),
-  DetectLibcxxIncludePath(SysRoot + "/usr/include/c++") };
+  DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/local/include/c++"),
+  DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/include/c++") };
   for (const auto  : LibCXXIncludePathCandidates) {
 if (IncludePath.empty() || !getVFS().exists(IncludePath))
   continue;


Index: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp
@@ -865,12 +865,13 @@
   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
 }
 
-static std::string DetectLibcxxIncludePath(StringRef base) {
+static std::string DetectLibcxxIncludePath(llvm::vfs::FileSystem ,
+   StringRef base) {
   std::error_code EC;
   int MaxVersion = 0;
   std::string MaxVersionString = "";
-  for (llvm::sys::fs::directory_iterator LI(base, EC), LE; !EC && LI != LE;
-   LI = LI.increment(EC)) {
+  for (llvm::vfs::directory_iterator LI = vfs.dir_begin(base, EC), LE;
+   !EC && LI != LE; LI = LI.increment(EC)) {
 StringRef VersionText = llvm::sys::path::filename(LI->path());
 int Version;
 if (VersionText[0] == 'v' &&
@@ -888,12 +889,12 @@
   llvm::opt::ArgStringList ) const {
   const std::string& SysRoot = computeSysRoot();
   const std::string LibCXXIncludePathCandidates[] = {
-  DetectLibcxxIncludePath(getDriver().Dir + "/../include/c++"),
+  DetectLibcxxIncludePath(getVFS(), getDriver().Dir + "/../include/c++"),
   // If this is a development, non-installed, clang, libcxx will
   // not be found at ../include/c++ but it likely to be found at
   // one of the following two locations:
-  DetectLibcxxIncludePath(SysRoot + "/usr/local/include/c++"),
-  DetectLibcxxIncludePath(SysRoot + "/usr/include/c++") };
+  DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/local/include/c++"),
+  DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/include/c++") };
   for (const auto  : LibCXXIncludePathCandidates) {
 if (IncludePath.empty() || !getVFS().exists(IncludePath))
   continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63663: [clang-doc] Add html links to references

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

I've also just noticed that the YAML generator is missing from this -- could 
you add the `Path` fields to hat output as well?




Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:213
+  llvm::SmallString<128> Path = Type.Path;
+  llvm::sys::path::append(Path, Type.Name + ".html");
+  return genLink(Type.Name, Path);

Now that the paths aren't absolute, you'll need to make this relative to the 
current path (something like 
https://github.com/llvm/llvm-project/blob/master/clang/lib/Lex/PPLexerChange.cpp#L201)


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

https://reviews.llvm.org/D63663



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

[PATCH] D63663: [clang-doc] Add html links to references

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:206
+  llvm::SmallString<128> Path = Type.Path;
+  ::llvm::sys::path::append(Path, Type.Name + ".html");
+  return genLink(Type.Name, Path);

You shouldn't need the prefixed `::`



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:466
   default:
 llvm_unreachable("Invalid reference type");
   }

While you're here, can you update this error to something like "Invalid 
reference type for parent namespace"?



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:567
+  ParentI->Namespace = llvm::SmallVector(
+  Enum.Namespace.begin() + 1, Enum.Namespace.end());
+  ParentI->Path = getInfoOutputFile(OutDirectory, ParentI->Namespace);

nit: use `++Enum.Namespace.begin()`



Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:219
 doc::Info *I = Reduced.get().get();
-
-auto InfoPath = getInfoOutputFile(OutDirectory, I->Namespace,
-  I->extractName(), "." + Format);
+auto InfoPath = getInfoOutputFile(I->Path, I->extractName(), "." + Format);
 if (!InfoPath) {

Could it add the OutDirectory here, and just store the relative path in 
`I->Path`? Since `OutDirectory` is constant throughout. You also wouldn't have 
to plumb it through all the serialize functions.


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

https://reviews.llvm.org/D63663



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

[PATCH] D63857: [clang-doc] Add a structured HTML generator

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM, unless Jake has further comments.




Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:503
+  }
+  // std::move(Nodes.begin(), Nodes.end(), 
std::back_inserter(MainContentNode));
+

Remove this


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

https://reviews.llvm.org/D63857



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

[PATCH] D64381: Update libc++ include path detection to use VFS on Linux

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: dlj, phosek.
Herald added a reviewer: EricWF.

The DetectLibcxxIncludePath function had been using 
llvm::sys::fs::directory_iterator, and this updates it to use 
llvm::vfs::directory_iterator.


https://reviews.llvm.org/D64381

Files:
  clang/lib/Driver/ToolChains/Linux.cpp


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -865,12 +865,12 @@
   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
 }
 
-static std::string DetectLibcxxIncludePath(StringRef base) {
+static std::string DetectLibcxxIncludePath(StringRef base, 
llvm::vfs::FileSystem& vfs) {
   std::error_code EC;
   int MaxVersion = 0;
   std::string MaxVersionString = "";
-  for (llvm::sys::fs::directory_iterator LI(base, EC), LE; !EC && LI != LE;
-   LI = LI.increment(EC)) {
+  for (llvm::vfs::directory_iterator LI = vfs.dir_begin(base, EC), LE;
+   !EC && LI != LE; LI = LI.increment(EC)) {
 StringRef VersionText = llvm::sys::path::filename(LI->path());
 int Version;
 if (VersionText[0] == 'v' &&
@@ -888,12 +888,12 @@
   llvm::opt::ArgStringList ) const {
   const std::string& SysRoot = computeSysRoot();
   const std::string LibCXXIncludePathCandidates[] = {
-  DetectLibcxxIncludePath(getDriver().Dir + "/../include/c++"),
+  DetectLibcxxIncludePath(getDriver().Dir + "/../include/c++", getVFS()),
   // If this is a development, non-installed, clang, libcxx will
   // not be found at ../include/c++ but it likely to be found at
   // one of the following two locations:
-  DetectLibcxxIncludePath(SysRoot + "/usr/local/include/c++"),
-  DetectLibcxxIncludePath(SysRoot + "/usr/include/c++") };
+  DetectLibcxxIncludePath(SysRoot + "/usr/local/include/c++", getVFS()),
+  DetectLibcxxIncludePath(SysRoot + "/usr/include/c++", getVFS()) };
   for (const auto  : LibCXXIncludePathCandidates) {
 if (IncludePath.empty() || !getVFS().exists(IncludePath))
   continue;


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -865,12 +865,12 @@
   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
 }
 
-static std::string DetectLibcxxIncludePath(StringRef base) {
+static std::string DetectLibcxxIncludePath(StringRef base, llvm::vfs::FileSystem& vfs) {
   std::error_code EC;
   int MaxVersion = 0;
   std::string MaxVersionString = "";
-  for (llvm::sys::fs::directory_iterator LI(base, EC), LE; !EC && LI != LE;
-   LI = LI.increment(EC)) {
+  for (llvm::vfs::directory_iterator LI = vfs.dir_begin(base, EC), LE;
+   !EC && LI != LE; LI = LI.increment(EC)) {
 StringRef VersionText = llvm::sys::path::filename(LI->path());
 int Version;
 if (VersionText[0] == 'v' &&
@@ -888,12 +888,12 @@
   llvm::opt::ArgStringList ) const {
   const std::string& SysRoot = computeSysRoot();
   const std::string LibCXXIncludePathCandidates[] = {
-  DetectLibcxxIncludePath(getDriver().Dir + "/../include/c++"),
+  DetectLibcxxIncludePath(getDriver().Dir + "/../include/c++", getVFS()),
   // If this is a development, non-installed, clang, libcxx will
   // not be found at ../include/c++ but it likely to be found at
   // one of the following two locations:
-  DetectLibcxxIncludePath(SysRoot + "/usr/local/include/c++"),
-  DetectLibcxxIncludePath(SysRoot + "/usr/include/c++") };
+  DetectLibcxxIncludePath(SysRoot + "/usr/local/include/c++", getVFS()),
+  DetectLibcxxIncludePath(SysRoot + "/usr/include/c++", getVFS()) };
   for (const auto  : LibCXXIncludePathCandidates) {
 if (IncludePath.empty() || !getVFS().exists(IncludePath))
   continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63911: [clang-doc] Serialize child namespaces and records

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364963: [clang-doc] Serialize child namespaces and records 
(authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63911?vs=207605=207612#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63911

Files:
  clang-tools-extra/trunk/clang-doc/Mapper.cpp
  clang-tools-extra/trunk/clang-doc/Serialize.cpp
  clang-tools-extra/trunk/clang-doc/Serialize.h
  clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
@@ -130,11 +130,12 @@
 
   ASSERT_EQ(Expected->ChildNamespaces.size(), Actual->ChildNamespaces.size());
   for (size_t Idx = 0; Idx < Actual->ChildNamespaces.size(); ++Idx)
-EXPECT_EQ(Expected->ChildNamespaces[Idx], Actual->ChildNamespaces[Idx]);
+CheckReference(Expected->ChildNamespaces[Idx],
+   Actual->ChildNamespaces[Idx]);
 
   ASSERT_EQ(Expected->ChildRecords.size(), Actual->ChildRecords.size());
   for (size_t Idx = 0; Idx < Actual->ChildRecords.size(); ++Idx)
-EXPECT_EQ(Expected->ChildRecords[Idx], Actual->ChildRecords[Idx]);
+CheckReference(Expected->ChildRecords[Idx], Actual->ChildRecords[Idx]);
 
   ASSERT_EQ(Expected->ChildFunctions.size(), Actual->ChildFunctions.size());
   for (size_t Idx = 0; Idx < Actual->ChildFunctions.size(); ++Idx)
@@ -167,7 +168,7 @@
 
   ASSERT_EQ(Expected->ChildRecords.size(), Actual->ChildRecords.size());
   for (size_t Idx = 0; Idx < Actual->ChildRecords.size(); ++Idx)
-EXPECT_EQ(Expected->ChildRecords[Idx], Actual->ChildRecords[Idx]);
+CheckReference(Expected->ChildRecords[Idx], Actual->ChildRecords[Idx]);
 
   ASSERT_EQ(Expected->ChildFunctions.size(), Actual->ChildFunctions.size());
   for (size_t Idx = 0; Idx < Actual->ChildFunctions.size(); ++Idx)
Index: clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
@@ -35,48 +35,30 @@
   ClangDocSerializeTestVisitor(EmittedInfoList , bool Public)
   : EmittedInfos(EmittedInfos), Public(Public) {}
 
-  bool VisitNamespaceDecl(const NamespaceDecl *D) {
+  template  bool mapDecl(const T *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
  /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
+if (I.first)
+  EmittedInfos.emplace_back(std::move(I.first));
+if (I.second)
+  EmittedInfos.emplace_back(std::move(I.second));
 return true;
   }
 
+  bool VisitNamespaceDecl(const NamespaceDecl *D) { return mapDecl(D); }
+
   bool VisitFunctionDecl(const FunctionDecl *D) {
 // Don't visit CXXMethodDecls twice
 if (dyn_cast(D))
   return true;
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
+return mapDecl(D);
   }
 
-  bool VisitCXXMethodDecl(const CXXMethodDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitCXXMethodDecl(const CXXMethodDecl *D) { return mapDecl(D); }
 
-  bool VisitRecordDecl(const RecordDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitRecordDecl(const RecordDecl *D) { return mapDecl(D); }
 
-  bool VisitEnumDecl(const EnumDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitEnumDecl(const EnumDecl *D) { return mapDecl(D); }
 };
 
 void ExtractInfosFromCode(StringRef Code, size_t NumExpectedInfos, bool Public,
@@ -101,19 +83,19 @@
 // Test serialization of namespace declarations.
 TEST(SerializeTest, emitNamespaceInfo) {
   EmittedInfoList Infos;
-  ExtractInfosFromCode("namespace A { namespace B { void f() {} } }", 3,
+  ExtractInfosFromCode("namespace A { namespace B { void f() {} } }", 5,
/*Public=*/false, Infos);
 
   

[PATCH] D63962: [clang-doc] Fix segfault in comment sorting

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364949: [clang-doc] Fix segfault in comment sorting 
(authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63962?vs=207170=207591#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63962

Files:
  clang-tools-extra/trunk/clang-doc/Representation.h


Index: clang-tools-extra/trunk/clang-doc/Representation.h
===
--- clang-tools-extra/trunk/clang-doc/Representation.h
+++ clang-tools-extra/trunk/clang-doc/Representation.h
@@ -75,15 +75,16 @@
  Other.ParamName, Other.CloseName, Other.SelfClosing,
  Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
 
-if (FirstCI < SecondCI ||
-(FirstCI == SecondCI && Children.size() < Other.Children.size()))
+if (FirstCI < SecondCI)
   return true;
 
-if (FirstCI > SecondCI || Children.size() > Other.Children.size())
-  return false;
+if (FirstCI == SecondCI) {
+  return std::lexicographical_compare(
+  Children.begin(), Children.end(), Other.Children.begin(),
+  Other.Children.end(), llvm::deref());
+}
 
-return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
-  llvm::deref{});
+return false;
   }
 
   SmallString<16>


Index: clang-tools-extra/trunk/clang-doc/Representation.h
===
--- clang-tools-extra/trunk/clang-doc/Representation.h
+++ clang-tools-extra/trunk/clang-doc/Representation.h
@@ -75,15 +75,16 @@
  Other.ParamName, Other.CloseName, Other.SelfClosing,
  Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
 
-if (FirstCI < SecondCI ||
-(FirstCI == SecondCI && Children.size() < Other.Children.size()))
+if (FirstCI < SecondCI)
   return true;
 
-if (FirstCI > SecondCI || Children.size() > Other.Children.size())
-  return false;
+if (FirstCI == SecondCI) {
+  return std::lexicographical_compare(
+  Children.begin(), Children.end(), Other.Children.begin(),
+  Other.Children.end(), llvm::deref());
+}
 
-return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
-  llvm::deref{});
+return false;
   }
 
   SmallString<16>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63962: [clang-doc] Fix segfault in comment sorting

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added a reviewer: jakehehrlich.
juliehockett added a project: clang-tools-extra.

https://reviews.llvm.org/D63962

Files:
  clang-tools-extra/clang-doc/Representation.h


Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -75,15 +75,16 @@
  Other.ParamName, Other.CloseName, Other.SelfClosing,
  Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
 
-if (FirstCI < SecondCI ||
-(FirstCI == SecondCI && Children.size() < Other.Children.size()))
+if (FirstCI < SecondCI)
   return true;
 
-if (FirstCI > SecondCI || Children.size() > Other.Children.size())
-  return false;
+if (FirstCI == SecondCI) {
+  return std::lexicographical_compare(
+  Children.begin(), Children.end(), Other.Children.begin(),
+  Other.Children.end(), llvm::deref());
+}
 
-return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
-  llvm::deref{});
+return false;
   }
 
   SmallString<16>


Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -75,15 +75,16 @@
  Other.ParamName, Other.CloseName, Other.SelfClosing,
  Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
 
-if (FirstCI < SecondCI ||
-(FirstCI == SecondCI && Children.size() < Other.Children.size()))
+if (FirstCI < SecondCI)
   return true;
 
-if (FirstCI > SecondCI || Children.size() > Other.Children.size())
-  return false;
+if (FirstCI == SecondCI) {
+  return std::lexicographical_compare(
+  Children.begin(), Children.end(), Other.Children.begin(),
+  Other.Children.end(), llvm::deref());
+}
 
-return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
-  llvm::deref{});
+return false;
   }
 
   SmallString<16>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63180: [clang-doc] Adds HTML generator

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

In D63180#1562982 , @jakehehrlich 
wrote:

> I'm getting confused in all the different patches. I reviewed the HTML 
> generator one like yesterday and was awaiting changes. Doesn't this do the 
> same thing?


Diego, why don't you just combine this patch with the structured HTML patch? 
Would lead to less confusion and churn.


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

https://reviews.llvm.org/D63180



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

[PATCH] D63911: [clang-doc] Serialize child namespaces and records

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:366
+
+  if (I->Namespace[0].RefType == InfoType::IT_namespace) {
+auto Parent = llvm::make_unique();

Can you make this a switch statement, doing the appropriate things for 
`IT_record` and `IT_namespace` and using `llvm_unreachable("Invalid reference 
type")` for all other possible values? This will help prevent silent 
regressions down the line.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:374
+
+  // At this point any parent will be a RecordInfo
+  auto Parent = llvm::make_unique();

nit: generally try to make comments full sentences



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:398
   I->ChildFunctions.emplace_back(std::move(Func));
-  return std::unique_ptr{std::move(I)};
+  // Info es wrapped in its parent scope so it's returned in the second 
position
+  return {nullptr, std::unique_ptr{std::move(I)}};

nit: spelling, here and below


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

https://reviews.llvm.org/D63911



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

[PATCH] D52847: [clang-doc] Handle anonymous namespaces

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364674: [clang-doc] Handle anonymous namespaces (authored by 
juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52847?vs=206979=207127#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D52847

Files:
  clang-tools-extra/trunk/clang-doc/Representation.cpp
  clang-tools-extra/trunk/clang-doc/Representation.h
  clang-tools-extra/trunk/clang-doc/Serialize.cpp
  clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
@@ -131,6 +131,7 @@
 
   NamespaceInfo *A = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedA(EmptySID);
+  ExpectedA.Name = "@nonymous_namespace";
   CheckNamespaceInfo(, A);
 }
 
Index: clang-tools-extra/trunk/clang-doc/Representation.h
===
--- clang-tools-extra/trunk/clang-doc/Representation.h
+++ clang-tools-extra/trunk/clang-doc/Representation.h
@@ -223,6 +223,8 @@
   void mergeBase(Info &);
   bool mergeable(const Info );
 
+  llvm::SmallString<16> extractName();
+
   // Returns a reference to the parent scope (that is, the immediate parent
   // namespace or class in which this decl resides).
   llvm::Expected getEnclosingScope();
Index: clang-tools-extra/trunk/clang-doc/Representation.cpp
===
--- clang-tools-extra/trunk/clang-doc/Representation.cpp
+++ clang-tools-extra/trunk/clang-doc/Representation.cpp
@@ -21,6 +21,7 @@
 //===--===//
 #include "Representation.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace doc {
@@ -194,5 +195,37 @@
   SymbolInfo::merge(std::move(Other));
 }
 
+llvm::SmallString<16> Info::extractName() {
+  if (!Name.empty())
+return Name;
+
+  switch (IT) {
+  case InfoType::IT_namespace:
+// Cover the case where the project contains a base namespace called
+// 'GlobalNamespace' (i.e. a namespace at the same level as the global
+// namespace, which would conflict with the hard-coded global namespace name
+// below.)
+if (Name == "GlobalNamespace" && Namespace.empty())
+  return llvm::SmallString<16>("@GlobalNamespace");
+// The case of anonymous namespaces is taken care of in serialization,
+// so here we can safely assume an unnamed namespace is the global
+// one.
+return llvm::SmallString<16>("GlobalNamespace");
+  case InfoType::IT_record:
+return llvm::SmallString<16>("@nonymous_record_" +
+ toHex(llvm::toStringRef(USR)));
+  case InfoType::IT_enum:
+return llvm::SmallString<16>("@nonymous_enum_" +
+ toHex(llvm::toStringRef(USR)));
+  case InfoType::IT_function:
+return llvm::SmallString<16>("@nonymous_function_" +
+ toHex(llvm::toStringRef(USR)));
+  case InfoType::IT_default:
+return llvm::SmallString<16>("@nonymous_" + toHex(llvm::toStringRef(USR)));
+  }
+  llvm_unreachable("Invalid InfoType.");
+  return llvm::SmallString<16>("");
+}
+
 } // namespace doc
 } // namespace clang
Index: clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
@@ -132,9 +132,6 @@
   if (CreateDirectory(Path))
 return llvm::make_error("Unable to create directory.\n",
llvm::inconvertibleErrorCode());
-
-  if (Name.empty())
-Name = "GlobalNamespace";
   llvm::sys::path::append(Path, Name + Ext);
   return Path;
 }
@@ -222,8 +219,8 @@
 
 doc::Info *I = Reduced.get().get();
 
-auto InfoPath =
-getInfoOutputFile(OutDirectory, I->Namespace, I->Name, "." + Format);
+auto InfoPath = getInfoOutputFile(OutDirectory, I->Namespace,
+  I->extractName(), "." + Format);
 if (!InfoPath) {
   llvm::errs() << toString(InfoPath.takeError()) << "\n";
   continue;
Index: clang-tools-extra/trunk/clang-doc/Serialize.cpp
===
--- clang-tools-extra/trunk/clang-doc/Serialize.cpp
+++ clang-tools-extra/trunk/clang-doc/Serialize.cpp
@@ -270,13 +270,19 @@
 template 
 static void
 populateParentNamespaces(llvm::SmallVector ,
- 

[PATCH] D62970: [clang-doc] De-duplicate comments and locations

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364670: [clang-doc] De-duplicate comments and locations 
(authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62970?vs=206570=207111#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62970

Files:
  clang-tools-extra/trunk/clang-doc/Representation.cpp
  clang-tools-extra/trunk/clang-doc/Representation.h
  clang-tools-extra/trunk/unittests/clang-doc/MergeTest.cpp

Index: clang-tools-extra/trunk/unittests/clang-doc/MergeTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/MergeTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/MergeTest.cpp
@@ -159,15 +159,37 @@
   One.IsMethod = true;
   One.Parent = Reference(EmptySID, "Parent", InfoType::IT_namespace);
 
+  One.Description.emplace_back();
+  auto OneFullComment = ();
+  OneFullComment->Kind = "FullComment";
+  auto OneParagraphComment = llvm::make_unique();
+  OneParagraphComment->Kind = "ParagraphComment";
+  auto OneTextComment = llvm::make_unique();
+  OneTextComment->Kind = "TextComment";
+  OneTextComment->Text = "This is a text comment.";
+  OneParagraphComment->Children.push_back(std::move(OneTextComment));
+  OneFullComment->Children.push_back(std::move(OneParagraphComment));
+
   FunctionInfo Two;
   Two.Name = "f";
   Two.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  Two.Loc.emplace_back(20, llvm::SmallString<16>{"test.cpp"});
+  Two.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   Two.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   Two.Params.emplace_back("int", "P");
 
+  Two.Description.emplace_back();
+  auto TwoFullComment = ();
+  TwoFullComment->Kind = "FullComment";
+  auto TwoParagraphComment = llvm::make_unique();
+  TwoParagraphComment->Kind = "ParagraphComment";
+  auto TwoTextComment = llvm::make_unique();
+  TwoTextComment->Kind = "TextComment";
+  TwoTextComment->Text = "This is a text comment.";
+  TwoParagraphComment->Children.push_back(std::move(TwoTextComment));
+  TwoFullComment->Children.push_back(std::move(TwoParagraphComment));
+
   std::vector> Infos;
   Infos.emplace_back(llvm::make_unique(std::move(One)));
   Infos.emplace_back(llvm::make_unique(std::move(Two)));
@@ -178,13 +200,23 @@
 
   Expected->DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   Expected->Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
-  Expected->Loc.emplace_back(20, llvm::SmallString<16>{"test.cpp"});
 
   Expected->ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   Expected->Params.emplace_back("int", "P");
   Expected->IsMethod = true;
   Expected->Parent = Reference(EmptySID, "Parent", InfoType::IT_namespace);
 
+  Expected->Description.emplace_back();
+  auto ExpectedFullComment = >Description.back();
+  ExpectedFullComment->Kind = "FullComment";
+  auto ExpectedParagraphComment = llvm::make_unique();
+  ExpectedParagraphComment->Kind = "ParagraphComment";
+  auto ExpectedTextComment = llvm::make_unique();
+  ExpectedTextComment->Kind = "TextComment";
+  ExpectedTextComment->Text = "This is a text comment.";
+  ExpectedParagraphComment->Children.push_back(std::move(ExpectedTextComment));
+  ExpectedFullComment->Children.push_back(std::move(ExpectedParagraphComment));
+
   auto Actual = mergeInfos(Infos);
   assert(Actual);
   CheckFunctionInfo(InfoAsFunction(Expected.get()),
Index: clang-tools-extra/trunk/clang-doc/Representation.h
===
--- clang-tools-extra/trunk/clang-doc/Representation.h
+++ clang-tools-extra/trunk/clang-doc/Representation.h
@@ -46,6 +46,45 @@
   CommentInfo() = default;
   CommentInfo(CommentInfo ) = delete;
   CommentInfo(CommentInfo &) = default;
+  CommentInfo =(CommentInfo &) = default;
+
+  bool operator==(const CommentInfo ) const {
+auto FirstCI = std::tie(Kind, Text, Name, Direction, ParamName, CloseName,
+SelfClosing, Explicit, AttrKeys, AttrValues, Args);
+auto SecondCI =
+std::tie(Other.Kind, Other.Text, Other.Name, Other.Direction,
+ Other.ParamName, Other.CloseName, Other.SelfClosing,
+ Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
+
+if (FirstCI != SecondCI || Children.size() != Other.Children.size())
+  return false;
+
+return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
+  llvm::deref{});
+  }
+
+  // This operator is used to sort a vector of CommentInfos.
+  // No specific order (attributes more important than others) is required. Any
+  // sort is enough, the order is only needed to call std::unique after sorting
+  // the vector.
+  bool operator<(const 

[PATCH] D63734: Update CODE_OWNERS.txt for clang-doc

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364663: Update CODE_OWNERS.txt for clang-doc (authored by 
juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63734?vs=206278=207095#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63734

Files:
  clang-tools-extra/trunk/CODE_OWNERS.TXT


Index: clang-tools-extra/trunk/CODE_OWNERS.TXT
===
--- clang-tools-extra/trunk/CODE_OWNERS.TXT
+++ clang-tools-extra/trunk/CODE_OWNERS.TXT
@@ -19,3 +19,7 @@
 N: Alexander Kornienko
 E: ale...@google.com
 D: clang-tidy
+
+N: Julie Hockett
+E: juliehock...@google.com
+D: clang-doc


Index: clang-tools-extra/trunk/CODE_OWNERS.TXT
===
--- clang-tools-extra/trunk/CODE_OWNERS.TXT
+++ clang-tools-extra/trunk/CODE_OWNERS.TXT
@@ -19,3 +19,7 @@
 N: Alexander Kornienko
 E: ale...@google.com
 D: clang-tidy
+
+N: Julie Hockett
+E: juliehock...@google.com
+D: clang-doc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52847: [clang-doc] Handle anonymous namespaces

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 206979.

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

https://reviews.llvm.org/D52847

Files:
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -131,6 +131,7 @@
 
   NamespaceInfo *A = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedA(EmptySID);
+  ExpectedA.Name = "@nonymous_namespace";
   CheckNamespaceInfo(, A);
 }
 
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -132,9 +132,6 @@
   if (CreateDirectory(Path))
 return llvm::make_error("Unable to create directory.\n",
llvm::inconvertibleErrorCode());
-
-  if (Name.empty())
-Name = "GlobalNamespace";
   llvm::sys::path::append(Path, Name + Ext);
   return Path;
 }
@@ -222,8 +219,8 @@
 
 doc::Info *I = Reduced.get().get();
 
-auto InfoPath =
-getInfoOutputFile(OutDirectory, I->Namespace, I->Name, "." + Format);
+auto InfoPath = getInfoOutputFile(OutDirectory, I->Namespace,
+  I->extractName(), "." + Format);
 if (!InfoPath) {
   llvm::errs() << toString(InfoPath.takeError()) << "\n";
   continue;
Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -270,13 +270,19 @@
 template 
 static void
 populateParentNamespaces(llvm::SmallVector ,
- const T *D) {
+ const T *D, bool ) {
   const auto *DC = dyn_cast(D);
   while ((DC = DC->getParent())) {
-if (const auto *N = dyn_cast(DC))
-  Namespaces.emplace_back(getUSRForDecl(N), N->getNameAsString(),
+if (const auto *N = dyn_cast(DC)) {
+  std::string Namespace;
+  if (N->isAnonymousNamespace()) {
+Namespace = "@nonymous_namespace";
+IsAnonymousNamespace = true;
+  } else
+Namespace = N->getNameAsString();
+  Namespaces.emplace_back(getUSRForDecl(N), Namespace,
   InfoType::IT_namespace);
-else if (const auto *N = dyn_cast(DC))
+} else if (const auto *N = dyn_cast(DC))
   Namespaces.emplace_back(getUSRForDecl(N), N->getNameAsString(),
   InfoType::IT_record);
 else if (const auto *N = dyn_cast(DC))
@@ -289,10 +295,11 @@
 }
 
 template 
-static void populateInfo(Info , const T *D, const FullComment *C) {
+static void populateInfo(Info , const T *D, const FullComment *C,
+ bool ) {
   I.USR = getUSRForDecl(D);
   I.Name = D->getNameAsString();
-  populateParentNamespaces(I.Namespace, D);
+  populateParentNamespaces(I.Namespace, D, IsInAnonymousNamespace);
   if (C) {
 I.Description.emplace_back();
 parseFullComment(C, I.Description.back());
@@ -301,8 +308,9 @@
 
 template 
 static void populateSymbolInfo(SymbolInfo , const T *D, const FullComment *C,
-   int LineNumber, StringRef Filename) {
-  populateInfo(I, D, C);
+   int LineNumber, StringRef Filename,
+   bool ) {
+  populateInfo(I, D, C, IsInAnonymousNamespace);
   if (D->isThisDeclarationADefinition())
 I.DefLoc.emplace(LineNumber, Filename);
   else
@@ -311,8 +319,9 @@
 
 static void populateFunctionInfo(FunctionInfo , const FunctionDecl *D,
  const FullComment *FC, int LineNumber,
- StringRef Filename) {
-  populateSymbolInfo(I, D, FC, LineNumber, Filename);
+ StringRef Filename,
+ bool ) {
+  populateSymbolInfo(I, D, FC, LineNumber, Filename, IsInAnonymousNamespace);
   if (const auto *T = getDeclForType(D->getReturnType())) {
 if (dyn_cast(T))
   I.ReturnType =
@@ -329,21 +338,28 @@
 std::unique_ptr emitInfo(const NamespaceDecl *D, const FullComment *FC,
int LineNumber, llvm::StringRef File,
bool PublicOnly) {
-  if (PublicOnly && ((D->isAnonymousNamespace()) ||
+  auto I = llvm::make_unique();
+  bool IsInAnonymousNamespace = false;
+  populateInfo(*I, D, FC, IsInAnonymousNamespace);
+  if (PublicOnly && ((IsInAnonymousNamespace || D->isAnonymousNamespace()) ||
  

[PATCH] D63911: [clang-doc] Serialize child namespaces and records

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:341
+  Parent->USR = ParentUSR;
+  Parent->ChildNamespaces.emplace_back(I->USR, I->Name, 
InfoType::IT_namespace);
+  return {std::unique_ptr{std::move(I)},

You're probably going to want the path in this too, here and below



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:398
   I->ChildFunctions.emplace_back(std::move(Func));
-  return std::unique_ptr{std::move(I)};
+  return {std::unique_ptr{std::move(I)}, nullptr};
 }

Maybe return the parent in the second position, and then comment about how the 
info itself is wrapped in its parent scope? That would make more sense 
logically, and they all end up in the same place. Same for methods/enums.



Comment at: clang-tools-extra/clang-doc/Serialize.h:30
 
-std::unique_ptr emitInfo(const NamespaceDecl *D, const FullComment *FC,
-   int LineNumber, StringRef File, bool 
PublicOnly);
-std::unique_ptr emitInfo(const RecordDecl *D, const FullComment *FC,
-   int LineNumber, StringRef File, bool 
PublicOnly);
-std::unique_ptr emitInfo(const EnumDecl *D, const FullComment *FC,
-   int LineNumber, StringRef File, bool 
PublicOnly);
-std::unique_ptr emitInfo(const FunctionDecl *D, const FullComment *FC,
-   int LineNumber, StringRef File, bool 
PublicOnly);
-std::unique_ptr emitInfo(const CXXMethodDecl *D, const FullComment *FC,
-   int LineNumber, StringRef File, bool 
PublicOnly);
+std::pair, std::unique_ptr>
+emitInfo(const NamespaceDecl *D, const FullComment *FC, int LineNumber,

Comment here what each of these represents, e.g. the first info contains the 
relevant information about the declaration, the second records the parent.



Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:41-44
+if (I.first)
+  EmittedInfos.emplace_back(std::move(I.first));
+if (I.second)
+  EmittedInfos.emplace_back(std::move(I.second));

Extract this logic into a mapDecl function, since it's repeated so much



Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:362
+  NamespaceInfo *ParentA = InfoAsNamespace(Infos[1].get());
+  NamespaceInfo ExpectedParentA(EmptySID);
+  ExpectedParentA.ChildRecords.emplace_back(Infos[0]->USR, "A",

Just use the EmptySID -- the checking function ignores the USRs since they can 
be system-dependent. Here and below.


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

https://reviews.llvm.org/D63911



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

[PATCH] D63857: [clang-doc] Structured HTML generator

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:21
 
+class HTMLTag {
+public:

Put the class definitions in an anonymous namespace 
(https://llvm.org/docs/CodingStandards.html#anonymous-namespaces)



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:24
+  // Any other tag can be added if required
+  enum Tag {
+meta,

Use `TagType` or some such to avoid confusion with the containing class



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:25-33
+meta,
+title,
+div,
+h1,
+h2,
+h3,
+p,

Enum values should be capitalized, and probably prefixed with HTML_ or TAG_ or 
some such useful prefix (e.g. TAG_META)



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:42
+
+  bool IsSelfClosing() const {
+switch (Value) {

For this and other functions, separate the implementation from the definition.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:121
+struct TagNode : public HTMLNode {
+  TagNode(HTMLTag Tag) : Tag(Tag) {
+InlineChildren = Tag.HasInlineChildren();

```TagNode(HTMLTag Tag) : Tag(Tag), InlineChildren(Tag.HasInlineChildren), 
SelfClosing(Tag.SelfClosing) {}```



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:412
+
+  F.Children.push_back(llvm::make_unique(HTMLTag::title, InfoTitle));
+  F.Children.push_back(std::move(MainContentNode));

use `emplace_back` here for instantiation



Comment at: clang-tools-extra/clang-doc/MDGenerator.cpp:31
 
-static void writeLine(const Twine , raw_ostream ) {
+void writeLine(const Twine , raw_ostream ) {
   OS << Text << "\n\n";

Can you re-static these?


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

https://reviews.llvm.org/D63857



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

[PATCH] D63734: Update CODE_OWNERS.txt for clang-doc

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

ping


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

https://reviews.llvm.org/D63734



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

[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:21
 
 namespace {
 

Eugene.Zelenko wrote:
> Anonymous namespace is used for functions when LLVM Coding Guidelines tells 
> using static for same purpose.
Actually, since these aren't used outside of this file, please use the `static` 
keyword instead of wrapping them in a namespace.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:58
+if (!R.Path.empty()) {
+  Stream << genLink(R.Name, R.Path + "/" + R.Name + ".html");
+} else {

Use llvm::sys::path to allow for other platforms that use a different separator.



Comment at: clang-tools-extra/clang-doc/MDGenerator.cpp:21
 
-namespace {
+namespace md_generator {
 

Same here, just use static.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:31
+// Example: Given the below, the  path for class C will be <
+// root>/A/B/C
+//

/A/B/C.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:230
   if (const auto *N = dyn_cast(T)) {
 I.Members.emplace_back(getUSRForDecl(T), N->getNameAsString(),
InfoType::IT_enum, F->getNameAsString(),

Also for members



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:255
   if (const auto *N = dyn_cast(T)) {
 I.Params.emplace_back(getUSRForDecl(N), N->getNameAsString(),
   InfoType::IT_enum, P->getNameAsString());

Also for param references, if applicable



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:308-310
+if (const auto *P = getDeclForType(B.getType()))
+  I.VirtualParents.emplace_back(getUSRForDecl(P), P->getNameAsString(),
+InfoType::IT_record);

Can we do the path construction for virtual bases as well, so they can be 
linked too?



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:341-350
   if (const auto *T = getDeclForType(D->getReturnType())) {
 if (dyn_cast(T))
   I.ReturnType =
   TypeInfo(getUSRForDecl(T), T->getNameAsString(), InfoType::IT_enum);
 else if (dyn_cast(T))
   I.ReturnType =
   TypeInfo(getUSRForDecl(T), T->getNameAsString(), 
InfoType::IT_record);

Add path information to these references as well



Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:145-146
   void ProtectedMethod();
-};)raw", 3, /*Public=*/false, Infos);
+};)raw",
+   3, /*Public=*/false, Infos);
 

formatting change?


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

https://reviews.llvm.org/D63663



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

[PATCH] D63367: [clang-doc] Add basic support for templates and typedef

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364222: [clang-doc] Add basic support for templates and 
typedef (authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63367?vs=205169=206282#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63367

Files:
  clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
  clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
  clang-tools-extra/trunk/clang-doc/Representation.h
  clang-tools-extra/trunk/clang-doc/Serialize.cpp
  clang-tools-extra/trunk/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
@@ -151,6 +151,8 @@
 
   EXPECT_EQ(Expected->TagType, Actual->TagType);
 
+  EXPECT_EQ(Expected->IsTypeDef, Actual->IsTypeDef);
+
   ASSERT_EQ(Expected->Members.size(), Actual->Members.size());
   for (size_t Idx = 0; Idx < Actual->Members.size(); ++Idx)
 EXPECT_EQ(Expected->Members[Idx], Actual->Members[Idx]);
Index: clang-tools-extra/trunk/unittests/clang-doc/BitcodeTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/BitcodeTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/BitcodeTest.cpp
@@ -80,6 +80,7 @@
 
   I.Members.emplace_back("int", "X", AccessSpecifier::AS_private);
   I.TagType = TagTypeKind::TTK_Class;
+  I.IsTypeDef = true;
   I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record);
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
 
Index: clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
@@ -142,7 +142,15 @@
   E() {}
 protected:
   void ProtectedMethod();
-};)raw", 3, /*Public=*/false, Infos);
+};
+template 
+struct F {
+  void TemplateMethod();
+};
+template <>
+void F::TemplateMethod();
+typedef struct {} G;)raw",
+   7, /*Public=*/false, Infos);
 
   RecordInfo *E = InfoAsRecord(Infos[0].get());
   RecordInfo ExpectedE(EmptySID, "E");
@@ -176,6 +184,51 @@
   Method.IsMethod = true;
   ExpectedRecordWithMethod.ChildFunctions.emplace_back(std::move(Method));
   CheckRecordInfo(, RecordWithMethod);
+
+  RecordInfo *F = InfoAsRecord(Infos[3].get());
+  RecordInfo ExpectedF(EmptySID, "F");
+  ExpectedF.TagType = TagTypeKind::TTK_Struct;
+  ExpectedF.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  CheckRecordInfo(, F);
+
+  RecordInfo *RecordWithTemplateMethod = InfoAsRecord(Infos[4].get());
+  RecordInfo ExpectedRecordWithTemplateMethod(EmptySID);
+  FunctionInfo TemplateMethod;
+  TemplateMethod.Name = "TemplateMethod";
+  TemplateMethod.Parent = Reference(EmptySID, "F", InfoType::IT_record);
+  TemplateMethod.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
+  TemplateMethod.Loc.emplace_back(0, llvm::SmallString<16>{"test.cpp"});
+  TemplateMethod.Namespace.emplace_back(EmptySID, "F", InfoType::IT_record);
+  TemplateMethod.Access = AccessSpecifier::AS_public;
+  TemplateMethod.IsMethod = true;
+  ExpectedRecordWithTemplateMethod.ChildFunctions.emplace_back(
+  std::move(TemplateMethod));
+  CheckRecordInfo(, RecordWithTemplateMethod);
+
+  RecordInfo *TemplatedRecord = InfoAsRecord(Infos[5].get());
+  RecordInfo ExpectedTemplatedRecord(EmptySID);
+  FunctionInfo SpecializedTemplateMethod;
+  SpecializedTemplateMethod.Name = "TemplateMethod";
+  SpecializedTemplateMethod.Parent =
+  Reference(EmptySID, "F", InfoType::IT_record);
+  SpecializedTemplateMethod.ReturnType =
+  TypeInfo(EmptySID, "void", InfoType::IT_default);
+  SpecializedTemplateMethod.Loc.emplace_back(0,
+ llvm::SmallString<16>{"test.cpp"});
+  SpecializedTemplateMethod.Namespace.emplace_back(EmptySID, "F",
+   InfoType::IT_record);
+  SpecializedTemplateMethod.Access = AccessSpecifier::AS_public;
+  SpecializedTemplateMethod.IsMethod = true;
+  ExpectedTemplatedRecord.ChildFunctions.emplace_back(
+  std::move(SpecializedTemplateMethod));
+  CheckRecordInfo(, TemplatedRecord);
+
+  RecordInfo *G = InfoAsRecord(Infos[6].get());
+  RecordInfo ExpectedG(EmptySID, "G");
+  ExpectedG.TagType = TagTypeKind::TTK_Struct;
+  ExpectedG.DefLoc = Location(0, 

[PATCH] D63734: Update CODE_OWNERS.txt for clang-doc

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added a reviewer: klimek.
juliehockett added a project: clang-tools-extra.

https://reviews.llvm.org/D63734

Files:
  clang-tools-extra/CODE_OWNERS.TXT


Index: clang-tools-extra/CODE_OWNERS.TXT
===
--- clang-tools-extra/CODE_OWNERS.TXT
+++ clang-tools-extra/CODE_OWNERS.TXT
@@ -19,3 +19,7 @@
 N: Alexander Kornienko
 E: ale...@google.com
 D: clang-tidy
+
+N: Julie Hockett
+E: juliehock...@google.com
+D: clang-doc


Index: clang-tools-extra/CODE_OWNERS.TXT
===
--- clang-tools-extra/CODE_OWNERS.TXT
+++ clang-tools-extra/CODE_OWNERS.TXT
@@ -19,3 +19,7 @@
 N: Alexander Kornienko
 E: ale...@google.com
 D: clang-tidy
+
+N: Julie Hockett
+E: juliehock...@google.com
+D: clang-doc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63666: [clang-doc] Add templates to HTML generator

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:59
+  Items +=
+  applyHTMLTemplate(HTMLTemplates::BlockItem, M.str().str().c_str());
+}

Put `FIXME: Transition Members from llvm::SmallString to std::string` and such 
on this (and others)



Comment at: clang-tools-extra/clang-doc/HTMLTemplates.cpp:163-166
+// - Open command (string), use Name attribute of Info
+// - Content of comment (string), use concatenated return value of genHTML() 
for
+// each child of Info
+// - Close command (string), use CloseName attribute of Info

For verbatim, the open/close commands are simply specifiers for where the 
section starts and stops, and so don't need to be rendered.



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:307-308
+
+
+
+f

Emitting an extra div here?



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:313-314
+
+
+
+ Brief description.

Emitting extra div/p here? Add a check in the comment generation bit to ensure 
you're not emitting an empty string for ParagraphComments


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

https://reviews.llvm.org/D63666



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

[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

General design thought: could we move the path name construction to 
`Serialize.cpp`? Essentially, move the logic that builds the path name from the 
namespaces and name to the serialize step (you'll have to add the 
`OutDirectory` to the `ClangDocContext` so that you can access it in the 
mapper), and then the `GetInfoOuputFile()` function in `ClangDocMain.cpp` would 
take the `Info.Path` string as a parameter and do the directory construction 
and whatnot.

With that, you could then also add the `Path` field to `Reference`s, so that 
you wouldn't need to pass a map around in the generation phase. What do you 
think?


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

https://reviews.llvm.org/D63663



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

[PATCH] D63180: [clang-doc] Adds HTML generator

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:87
+std::string CloseTag = I.SelfClosing ? "/>" : ">";
+writeLine("<" + I.Name + Attrs.str() + CloseTag, OS);
+  } else if (I.Kind == "HTMLEndTagComment") {

You shouldn't be using writeLine here, since it wraps the tag in an extra  
element (which is okay for all the other pieces, but is weird on a specified 
HTML element.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:89
+  } else if (I.Kind == "HTMLEndTagComment") {
+writeLine("", OS);
+  } else if (I.Kind == "TextComment") {

Same as above



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:110
+for (const auto  : I.Members)
+  Members << "| " << N << " |\n";
+  writeLine(Members.str(), OS);

The pipes don't mean anything in HTML, so you can remove them.



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:214-218
+  HTML->Children.back()->Kind = "HTMLStartTagComment";
+  HTML->Children.back()->Name = "li";
+  HTML->Children.emplace_back(llvm::make_unique());
+  HTML->Children.back()->Kind = "TextComment";
+  HTML->Children.back()->Text = " Testing.";

For the sake of vaguely valid HTML in the test output, it'd be nice if we had 
an end tag for the  element as well.



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:287
+Defined at line 10 of test.cpp
+
+ Brief description.

Should the whole comment be wrapped in a  element?


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

https://reviews.llvm.org/D63180



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

[PATCH] D62437: [clang-tidy] Splits fuchsia-default-arguments

[Julie Hockett via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363712: [clang-tidy] Split fuchsia-default-arguments 
(authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62437?vs=204157=205402#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62437

Files:
  clang-tools-extra/trunk/clang-tidy/fuchsia/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsCallsCheck.cpp
  clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsCallsCheck.h
  clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsCheck.cpp
  clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsCheck.h
  
clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.cpp
  clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.h
  clang-tools-extra/trunk/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/fuchsia-default-arguments-calls.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/fuchsia-default-arguments-declarations.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/fuchsia-default-arguments.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  clang-tools-extra/trunk/test/clang-tidy/fuchsia-default-arguments-calls.cpp
  
clang-tools-extra/trunk/test/clang-tidy/fuchsia-default-arguments-declarations.cpp
  clang-tools-extra/trunk/test/clang-tidy/fuchsia-default-arguments.cpp

Index: clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.h
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.h
@@ -0,0 +1,34 @@
+//===--- DefaultArgumentsDeclarationsCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_DEFAULT_ARGUMENTS_DECLARATIONS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_DEFAULT_ARGUMENTS_DECLARATIONS_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+/// Default parameters are not allowed in declared functions.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-default-parameters.html
+class DefaultArgumentsDeclarationsCheck : public ClangTidyCheck {
+public:
+  DefaultArgumentsDeclarationsCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace fuchsia
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_DEFAULT_ARGUMENTS_DECLARATIONS_H
Index: clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.cpp
@@ -0,0 +1,54 @@
+//===--- DefaultArgumentsDeclarationsCheck.cpp - clang-tidy ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "DefaultArgumentsDeclarationsCheck.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+void DefaultArgumentsDeclarationsCheck::registerMatchers(MatchFinder *Finder) {
+  // Declaring default parameters is disallowed.
+  Finder->addMatcher(parmVarDecl(hasDefaultArgument()).bind("decl"), this);
+}
+
+void DefaultArgumentsDeclarationsCheck::check(
+const MatchFinder::MatchResult ) {
+  const auto *D = Result.Nodes.getNodeAs("decl");
+  if (!D)
+return;
+
+  SourceRange DefaultArgRange = D->getDefaultArgRange();
+
+  if (DefaultArgRange.getEnd() != D->getEndLoc())
+return;
+
+  if (DefaultArgRange.getBegin().isMacroID()) {
+diag(D->getBeginLoc(),
+ "declaring a parameter with a default argument is disallowed");
+return;
+  }
+
+  SourceLocation StartLocation =
+  

[PATCH] D63367: [clang-doc] Add basic support for templates and typedef

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/Representation.h:244
  // interface).
+  bool IsTypeDef = false; // Indicates if record was declared using typedef
   llvm::SmallVector

Run clang-format? Not totally sure what it'll do, but I think it should line up 
the comments.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:387
 
-  SymbolID ParentUSR = getUSRForDecl(D->getParent());
-  Func.Parent = Reference{ParentUSR, D->getParent()->getNameAsString(),
-  InfoType::IT_record};
+  const NamedDecl *Parent;
+  if (const auto *SD =

Generally, it's not a great idea to declare a pointer without initializing it. 
Either initialize it on declaration or initialize to nullptr.


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

https://reviews.llvm.org/D63367



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

[PATCH] D62970: [clang-doc] De-duplicate comments

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

In D62970#1533229 , @jakehehrlich 
wrote:

> Actually if we can make Description a hash set that would be best. I think 
> using std::set would require an awkward < operator to be defined. There 
> should be standard ways of getting hashes out of all the string and vector 
> types and you can use hash_combine to combine all of these into a single hash.


This SGTM -- this would actually make it a bit more deterministic in the 
ordering, since we'd no longer depend on the order of file load, even.




Comment at: clang-tools-extra/clang-doc/Representation.cpp:139-140
 DefLoc = std::move(Other.DefLoc);
   // Unconditionally extend the list of locations, since we want all of them.
   std::move(Other.Loc.begin(), Other.Loc.end(), std::back_inserter(Loc));
   mergeBase(std::move(Other));

While you're here, could you actually do the same for Location? That is, make 
it a hash set on `SymbolInfo` and dedup here?


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

https://reviews.llvm.org/D62970



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

[PATCH] D62970: [clang-doc] De-duplicate comments

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

Please add a test case to `unittests/clang-doc/MergeTest.cpp`.




Comment at: clang-tools-extra/clang-doc/Representation.cpp:124-127
+bool IsCommentUnique = std::find(Description.begin(), Description.end(),
+ Comment) == Description.end();
+if (IsCommentUnique)
+  Description.emplace_back(std::move(Comment));

```if (std::find(Description.begin(), Description.end(), Comment) == 
Description.end())
  Description.emplace_back(std::move(Comment));```



Comment at: clang-tools-extra/clang-doc/Representation.h:51-59
+bool AreEqual =
+Kind == Other.Kind && Text == Other.Text && Name == Other.Name &&
+Direction == Other.Direction && ParamName == Other.ParamName &&
+CloseName == Other.CloseName && SelfClosing == Other.SelfClosing &&
+Explicit == Other.Explicit && AttrKeys == Other.AttrKeys &&
+AttrValues == Other.AttrValues && Args == Other.Args &&
+Children.size() == Other.Children.size();

Use `std::tie` here (see `Reference::operator==` below as an example).


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

https://reviews.llvm.org/D62970



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

[PATCH] D62437: [clang-tidy] Splits fuchsia-default-arguments

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

Sorry for the delay!




Comment at: 
clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsCallsCheck.cpp:24
+  const auto *S = Result.Nodes.getNodeAs("stmt");
+  if (S == nullptr)
+return;

Just `if (!S)` should be sufficient here.



Comment at: 
clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.cpp:25
+  const auto *D = Result.Nodes.getNodeAs("decl");
+  if (D == nullptr)
+return;

Same here for the `if` as above.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:118-122
+- New :doc:`fuchsia-default-arguments-declarations
+  ` check.
+
+  Warns if a function or method is declared with default parameters.
+

Also include a line about the addition of fuchsia-default-arguments-calls and 
the removal of fuchsia-default-arguments.



Comment at: 
clang-tools-extra/test/clang-tidy/fuchsia-default-arguments-declarations.cpp:59-60
 
 int main() {
-  S s;
-  s.x();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: calling a function that uses a 
default argument is disallowed [fuchsia-default-arguments]
-  // CHECK-NOTES: [[@LINE-8]]:11: note: default parameter was declared here
-  // CHECK-NEXT: void S::x(int i = 12) {}
-  x();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: calling a function that uses a 
default argument is disallowed [fuchsia-default-arguments]
-  // CHECK-NOTES: [[@LINE-18]]:8: note: default parameter was declared here
-  // CHECK-NEXT: void x(int i = 12);
 }

Remove this


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

https://reviews.llvm.org/D62437



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

[PATCH] D62157: Remove explicit header-filter in run_clang_tidy.py

[Julie Hockett via Phabricator via cfe-commits]

juliehockett abandoned this revision.
juliehockett added a comment.

@hintonda Aha so it is :)


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

https://reviews.llvm.org/D62157



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

[PATCH] D62157: Remove explicit header-filter in run_clang_tidy.py

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: alexfh, hokein.
juliehockett added a project: clang-tools-extra.

This removes the else clause that adds a default `-header-filter` flag to 
clang-tidy invocations from the run_clang_tidy.py script. If this clause is 
present, any HeaderFilterRegex settings in the `.clang-tidy` file are ignored 
and would have to be explicitly passed to the script each time.


https://reviews.llvm.org/D62157

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -84,9 +84,6 @@
   start = [clang_tidy_binary]
   if header_filter is not None:
 start.append('-header-filter=' + header_filter)
-  else:
-# Show warnings in all in-project headers by default.
-start.append('-header-filter=^' + build_path + '/.*')
   if checks:
 start.append('-checks=' + checks)
   if tmpdir is not None:


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -84,9 +84,6 @@
   start = [clang_tidy_binary]
   if header_filter is not None:
 start.append('-header-filter=' + header_filter)
-  else:
-# Show warnings in all in-project headers by default.
-start.append('-header-filter=^' + build_path + '/.*')
   if checks:
 start.append('-checks=' + checks)
   if tmpdir is not None:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

This is also making our Mac builders fail on configuration, and looks like a 
directory wasn't included properly when a test moved to a subdirectory -- would 
you mind taking a look? The CMake command we're running is at the top of the 
attached log.

  CMake Error at 
/b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/unittests/xpc/CMakeLists.txt:11
 (add_extra_unittest):
Unknown CMake command "add_extra_unittest".

Full Log: 
https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8914851701929934272/+/steps/clang/0/steps/configure/0/stdout


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187



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

[PATCH] D59975: [fuchsia] Add clang-doc to Fuchsia distribution

[Julie Hockett via Phabricator via cfe-commits]

juliehockett closed this revision.
juliehockett added a comment.

Closed and submitted in r357275


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

https://reviews.llvm.org/D59975



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

[PATCH] D59974: [clang-doc] Build as clang_tool

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357274: [clang-doc] Build as clang_tool (authored by 
juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59974?vs=192759=192850#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59974

Files:
  clang-tools-extra/trunk/clang-doc/tool/CMakeLists.txt


Index: clang-tools-extra/trunk/clang-doc/tool/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-doc/tool/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-doc/tool/CMakeLists.txt
@@ -1,6 +1,6 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
-add_clang_executable(clang-doc
+add_clang_tool(clang-doc
   ClangDocMain.cpp
   )
 


Index: clang-tools-extra/trunk/clang-doc/tool/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-doc/tool/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-doc/tool/CMakeLists.txt
@@ -1,6 +1,6 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
-add_clang_executable(clang-doc
+add_clang_tool(clang-doc
   ClangDocMain.cpp
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59974: [clang-doc] Build as clang_tool

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: phosek, leonardchan.
juliehockett added a project: clang-tools-extra.
Herald added a subscriber: mgorny.

Instead of as clang_executable.


https://reviews.llvm.org/D59974

Files:
  clang-tools-extra/clang-doc/tool/CMakeLists.txt


Index: clang-tools-extra/clang-doc/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-doc/tool/CMakeLists.txt
+++ clang-tools-extra/clang-doc/tool/CMakeLists.txt
@@ -1,6 +1,6 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
-add_clang_executable(clang-doc
+add_clang_tool(clang-doc
   ClangDocMain.cpp
   )
 


Index: clang-tools-extra/clang-doc/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-doc/tool/CMakeLists.txt
+++ clang-tools-extra/clang-doc/tool/CMakeLists.txt
@@ -1,6 +1,6 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
-add_clang_executable(clang-doc
+add_clang_tool(clang-doc
   ClangDocMain.cpp
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56825: [CMake][Fuchsia] Disable modules for the second stage build

[Julie Hockett via Phabricator via cfe-commits]

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

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D56825



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

[PATCH] D52847: [clang-doc] Handle anonymous namespaces

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 180182.

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

https://reviews.llvm.org/D52847

Files:
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -132,6 +132,7 @@
 
   NamespaceInfo *A = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedA(EmptySID);
+  ExpectedA.Name = "@nonymous_namespace";
   CheckNamespaceInfo(, A);
 }
 
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -133,9 +133,6 @@
   if (CreateDirectory(Path))
 return llvm::make_error("Unable to create directory.\n",
llvm::inconvertibleErrorCode());
-
-  if (Name.empty())
-Name = "GlobalNamespace";
   llvm::sys::path::append(Path, Name + Ext);
   return Path;
 }
@@ -223,8 +220,8 @@
 
 doc::Info *I = Reduced.get().get();
 
-auto InfoPath =
-getInfoOutputFile(OutDirectory, I->Namespace, I->Name, "." + Format);
+auto InfoPath = getInfoOutputFile(OutDirectory, I->Namespace,
+  I->extractName(), "." + Format);
 if (!InfoPath) {
   llvm::errs() << toString(InfoPath.takeError()) << "\n";
   continue;
Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -268,13 +268,19 @@
 template 
 static void
 populateParentNamespaces(llvm::SmallVector ,
- const T *D) {
+ const T *D, bool ) {
   const auto *DC = dyn_cast(D);
   while ((DC = DC->getParent())) {
-if (const auto *N = dyn_cast(DC))
-  Namespaces.emplace_back(getUSRForDecl(N), N->getNameAsString(),
+if (const auto *N = dyn_cast(DC)) {
+  std::string Namespace;
+  if (N->isAnonymousNamespace()) {
+Namespace = "@nonymous_namespace";
+IsAnonymousNamespace = true;
+  } else
+Namespace = N->getNameAsString();
+  Namespaces.emplace_back(getUSRForDecl(N), Namespace,
   InfoType::IT_namespace);
-else if (const auto *N = dyn_cast(DC))
+} else if (const auto *N = dyn_cast(DC))
   Namespaces.emplace_back(getUSRForDecl(N), N->getNameAsString(),
   InfoType::IT_record);
 else if (const auto *N = dyn_cast(DC))
@@ -287,10 +293,11 @@
 }
 
 template 
-static void populateInfo(Info , const T *D, const FullComment *C) {
+static void populateInfo(Info , const T *D, const FullComment *C,
+ bool ) {
   I.USR = getUSRForDecl(D);
   I.Name = D->getNameAsString();
-  populateParentNamespaces(I.Namespace, D);
+  populateParentNamespaces(I.Namespace, D, IsInAnonymousNamespace);
   if (C) {
 I.Description.emplace_back();
 parseFullComment(C, I.Description.back());
@@ -299,8 +306,9 @@
 
 template 
 static void populateSymbolInfo(SymbolInfo , const T *D, const FullComment *C,
-   int LineNumber, StringRef Filename) {
-  populateInfo(I, D, C);
+   int LineNumber, StringRef Filename,
+   bool ) {
+  populateInfo(I, D, C, IsInAnonymousNamespace);
   if (D->isThisDeclarationADefinition())
 I.DefLoc.emplace(LineNumber, Filename);
   else
@@ -309,8 +317,9 @@
 
 static void populateFunctionInfo(FunctionInfo , const FunctionDecl *D,
  const FullComment *FC, int LineNumber,
- StringRef Filename) {
-  populateSymbolInfo(I, D, FC, LineNumber, Filename);
+ StringRef Filename,
+ bool ) {
+  populateSymbolInfo(I, D, FC, LineNumber, Filename, IsInAnonymousNamespace);
   if (const auto *T = getDeclForType(D->getReturnType())) {
 if (dyn_cast(T))
   I.ReturnType =
@@ -327,21 +336,28 @@
 std::unique_ptr emitInfo(const NamespaceDecl *D, const FullComment *FC,
int LineNumber, llvm::StringRef File,
bool PublicOnly) {
-  if (PublicOnly && ((D->isAnonymousNamespace()) ||
+  auto I = llvm::make_unique();
+  bool IsInAnonymousNamespace = false;
+  populateInfo(*I, D, FC, IsInAnonymousNamespace);
+  if (PublicOnly && ((IsInAnonymousNamespace || D->isAnonymousNamespace()) ||
  

[PATCH] D55848: [clang-tidy] Add export-fixes flag to clang-tidy-diff

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349930: [clang-tidy] Add export-fixes flag to 
clang-tidy-diff (authored by juliehockett, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55848?vs=178757=179301#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55848

Files:
  clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py


Index: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
@@ -57,6 +57,9 @@
   default='')
   parser.add_argument('-path', dest='build_path',
   help='Path used to read a compile command database.')
+  parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
+  help='Create a yaml file to store suggested fixes in, '
+  'which can be applied with clang-apply-replacements.')
   parser.add_argument('-extra-arg', dest='extra_arg',
   action='append', default=[],
   help='Additional argument to append to the compiler '
@@ -122,6 +125,8 @@
   command.append('-line-filter=' + quote + line_filter_json + quote)
   if args.fix:
 command.append('-fix')
+  if args.export_fixes:
+command.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
 command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:


Index: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
@@ -57,6 +57,9 @@
   default='')
   parser.add_argument('-path', dest='build_path',
   help='Path used to read a compile command database.')
+  parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
+  help='Create a yaml file to store suggested fixes in, '
+  'which can be applied with clang-apply-replacements.')
   parser.add_argument('-extra-arg', dest='extra_arg',
   action='append', default=[],
   help='Additional argument to append to the compiler '
@@ -122,6 +125,8 @@
   command.append('-line-filter=' + quote + line_filter_json + quote)
   if args.fix:
 command.append('-fix')
+  if args.export_fixes:
+command.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
 command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55848: [clang-tidy] Add export-fixes flag to clang-tidy-diff

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: aaron.ballman, hokein, alexfh.
juliehockett added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.

So that you can export the fixes generated.


https://reviews.llvm.org/D55848

Files:
  clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py


Index: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
@@ -57,6 +57,9 @@
   default='')
   parser.add_argument('-path', dest='build_path',
   help='Path used to read a compile command database.')
+  parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
+  help='Create a yaml file to store suggested fixes in, '
+  'which can be applied with clang-apply-replacements.')
   parser.add_argument('-extra-arg', dest='extra_arg',
   action='append', default=[],
   help='Additional argument to append to the compiler '
@@ -122,6 +125,8 @@
   command.append('-line-filter=' + quote + line_filter_json + quote)
   if args.fix:
 command.append('-fix')
+  if args.export_fixes:
+command.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
 command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:


Index: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
@@ -57,6 +57,9 @@
   default='')
   parser.add_argument('-path', dest='build_path',
   help='Path used to read a compile command database.')
+  parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
+  help='Create a yaml file to store suggested fixes in, '
+  'which can be applied with clang-apply-replacements.')
   parser.add_argument('-extra-arg', dest='extra_arg',
   action='append', default=[],
   help='Additional argument to append to the compiler '
@@ -122,6 +125,8 @@
   command.append('-line-filter=' + quote + line_filter_json + quote)
   if args.fix:
 command.append('-fix')
+  if args.export_fixes:
+command.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
 command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54168: [clang-tidy] Zircon fbl::move -> std::move

[Julie Hockett via Phabricator via cfe-commits]

juliehockett abandoned this revision.
juliehockett added a comment.

After a lot of discussion, we'll do this migration internally. Thanks for your 
comments!


https://reviews.llvm.org/D54168



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

[PATCH] D54169: [clang-tidy] Zircon ->

[Julie Hockett via Phabricator via cfe-commits]

juliehockett abandoned this revision.
juliehockett added a comment.

After a lot of discussion, we'll do this migration internally. Thanks for your 
comments!


https://reviews.llvm.org/D54169



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

[PATCH] D54169: [clang-tidy] Zircon ->

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47
+SrcMgr::CharacteristicKind FileType) {
+  if (FileName == "fbl/limits.h") {
+unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1;

aaron.ballman wrote:
> juliehockett wrote:
> > aaron.ballman wrote:
> > > juliehockett wrote:
> > > > aaron.ballman wrote:
> > > > > Does this also work on platforms where the path separator is `\` 
> > > > > instead of `/`? What about case insensitive file systems where it may 
> > > > > be spelled `LiMiTs.H`? Does this properly handle a case like:
> > > > > ```
> > > > > #define LIMITS "fbl/limits.h"
> > > > > #include LIMITS
> > > > > ```
> > > > > (Should add test cases for all of these scenarios.)
> > > > Since this is a migration for our own codebase, we know we don't have 
> > > > any code that uses any variation other than  and so 
> > > > hardcoding that is acceptable to us here.
> > > Then why should this check be a public one, rather than an internal check?
> > I explained this on the other one, but for completeness: 
> > 
> > 
> > So yes, this check is for a migration, and we would delete it once 
> > regressions weren't possible. We would like the suite to be in upstream, 
> > however, because we use the ToT llvm/clang/tools/etc, and don't want to 
> > have to fork just to use clang-tidy for this sort of thing. Since 
> > clang-tidy doesn't provide any way to have external checks to the tool 
> > itself, upstreaming is the most ideal option.
> > 
> > Orthogonal to our particular build setup, it'd also be nice to have an 
> > example of this sort of migration done by clang-tidy in-tree. There has 
> > been a lot of discussion recently about doing migrations with clang-tidy, 
> > but it's always describing an internal migration that uses a forked tree 
> > and a private suite of checks that can't be released.
> I don't think it's reasonable to expect the community to bear the maintenance 
> burden for internal-only checks for an organization. I definitely understand 
> the desire not to carry around a fork of clang-tidy for this, but that 
> doesn't seem like a good reason for us to spend cycles reviewing these 
> patches, maintaining them, and then eventually removing them.
> 
> We have some precedent in that we have clang-tidy checks for llvm coding 
> standards or google coding standards, etc but those are also used outside of 
> the particular community for which they're named. In this case, I don't think 
> the functionality is useful to anyone except Google. Is that correct?
That makes sense in terms of investment from you and others (though I do 
appreciate the effort you've put in for reviewing it thus far), but I (and my 
team) are willing to bear the burden of reviewing/maintaining/removing them. If 
clang-tidy provided a way to tack on a set of external checks without forking, 
we'd happily do that, but that's not really possible right now.

Would it be reasonable to do the review/maintenance/removal of these migration 
checks from within our team, such that we're not asking you to spend cycles on 
them?




https://reviews.llvm.org/D54169



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

[PATCH] D54169: [clang-tidy] Zircon ->

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47
+SrcMgr::CharacteristicKind FileType) {
+  if (FileName == "fbl/limits.h") {
+unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1;

aaron.ballman wrote:
> juliehockett wrote:
> > aaron.ballman wrote:
> > > Does this also work on platforms where the path separator is `\` instead 
> > > of `/`? What about case insensitive file systems where it may be spelled 
> > > `LiMiTs.H`? Does this properly handle a case like:
> > > ```
> > > #define LIMITS "fbl/limits.h"
> > > #include LIMITS
> > > ```
> > > (Should add test cases for all of these scenarios.)
> > Since this is a migration for our own codebase, we know we don't have any 
> > code that uses any variation other than  and so hardcoding 
> > that is acceptable to us here.
> Then why should this check be a public one, rather than an internal check?
I explained this on the other one, but for completeness: 


So yes, this check is for a migration, and we would delete it once regressions 
weren't possible. We would like the suite to be in upstream, however, because 
we use the ToT llvm/clang/tools/etc, and don't want to have to fork just to use 
clang-tidy for this sort of thing. Since clang-tidy doesn't provide any way to 
have external checks to the tool itself, upstreaming is the most ideal option.

Orthogonal to our particular build setup, it'd also be nice to have an example 
of this sort of migration done by clang-tidy in-tree. There has been a lot of 
discussion recently about doing migrations with clang-tidy, but it's always 
describing an internal migration that uses a forked tree and a private suite of 
checks that can't be released.


https://reviews.llvm.org/D54169



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

[PATCH] D54169: [clang-tidy] Zircon ->

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47
+SrcMgr::CharacteristicKind FileType) {
+  if (FileName == "fbl/limits.h") {
+unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1;

aaron.ballman wrote:
> Does this also work on platforms where the path separator is `\` instead of 
> `/`? What about case insensitive file systems where it may be spelled 
> `LiMiTs.H`? Does this properly handle a case like:
> ```
> #define LIMITS "fbl/limits.h"
> #include LIMITS
> ```
> (Should add test cases for all of these scenarios.)
Since this is a migration for our own codebase, we know we don't have any code 
that uses any variation other than  and so hardcoding that is 
acceptable to us here.



Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:70
+void FblLimitsCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(varDecl(hasType(cxxRecordDecl(allOf(
+ hasDeclContext(namespaceDecl(hasName("fbl"))),

aaron.ballman wrote:
> Can users specialize `fbl::numeric_limits` for custom integral types like 
> they can for `std::numeric_limits`? If so, that should maybe be covered with 
> a checker as well.
Technically, they could, but there are no instances of it in the codebase.


https://reviews.llvm.org/D54169



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

[PATCH] D54168: [clang-tidy] Zircon fbl::move -> std::move

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 172892.
juliehockett marked 2 inline comments as done.

https://reviews.llvm.org/D54168

Files:
  clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
  clang-tools-extra/clang-tidy/zircon/FblMoveCheck.cpp
  clang-tools-extra/clang-tidy/zircon/FblMoveCheck.h
  clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-move.rst
  clang-tools-extra/test/clang-tidy/zircon-fbl-move.cpp

Index: clang-tools-extra/test/clang-tidy/zircon-fbl-move.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-fbl-move.cpp
@@ -0,0 +1,37 @@
+// RUN: %check_clang_tidy %s zircon-fbl-move %t
+
+// CHECK-FIXES: #include 
+
+namespace fbl {
+
+void move(int i) {}
+
+} // namespace fbl
+
+namespace std {
+
+void move(int i) {}
+
+} // namespace std
+
+#define MOVE(i) fbl::move(i)
+// CHECK-NOTES: [[@LINE-1]]:17: warning: fbl::move is deprecated, use std::move instead
+// CHECK-FIXES: #define MOVE(i) std::move(i)
+
+int main() {
+  int i = 0;
+  fbl::move(i);
+  // CHECK-NOTES: [[@LINE-1]]:3: warning: fbl::move is deprecated, use std::move instead
+  // CHECK-FIXES: std::move(i)
+
+  int j = 0;
+  std::move(j);
+
+  void (*foo)(int);
+  foo = ::move;
+  // CHECK-NOTES: [[@LINE-1]]:10: warning: fbl::move is deprecated, use std::move instead
+  // CHECK-FIXES: foo = ::move
+
+  int k = 0;
+  MOVE(k);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-move.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-move.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - zircon-fbl-move
+
+zircon-fbl-move
+===
+
+This check is part of the migration checks for moving Zircon user code to use
+the C++ standard library.
+
+It suggests converting uses of ``fbl::move`` to ``std::move``, and suggests
+to include the  header.
+
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
@@ -254,4 +254,5 @@
readability-string-compare
readability-uniqueptr-delete-release
readability-uppercase-literal-suffix
+   zircon-fbl-move
zircon-temporary-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -182,6 +182,12 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- New :doc:`zircon-fbl-move
+  ` check.
+
+  Suggests converting uses of ``fbl::move`` to ``std::move``, and suggests
+  to include the  header.
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
+++ clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "FblMoveCheck.h"
 #include "TemporaryObjectsCheck.h"
 
 using namespace clang::ast_matchers;
@@ -22,6 +23,8 @@
 class ZirconModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"zircon-fbl-move");
 CheckFactories.registerCheck(
 "zircon-temporary-objects");
   }
Index: clang-tools-extra/clang-tidy/zircon/FblMoveCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/zircon/FblMoveCheck.h
@@ -0,0 +1,45 @@
+//===--- FblMoveCheck.h - clang-tidy *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ZIRCON_FBLINCLUDENEWCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ZIRCON_FBLINCLUDENEWCHECK_H
+
+#include "../ClangTidy.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang {
+namespace tidy {
+namespace zircon {
+
+/// Replace instances of `fbl::move` with `std::move`, and adds the
+/// `` header if a replacement occurs.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/zircon-fbl-move.html
+class FblMoveCheck : public ClangTidyCheck {
+public:
+  FblMoveCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context),
+   

[PATCH] D54169: [clang-tidy] Zircon ->

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 172891.
juliehockett marked 5 inline comments as done.

https://reviews.llvm.org/D54169

Files:
  clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
  clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp
  clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.h
  clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-limits.rst
  clang-tools-extra/test/clang-tidy/Inputs/zircon/fbl/limits.h
  clang-tools-extra/test/clang-tidy/Inputs/zircon/transitive.h
  clang-tools-extra/test/clang-tidy/zircon-fbl-limits-transitive.cpp
  clang-tools-extra/test/clang-tidy/zircon-fbl-limits.cpp

Index: clang-tools-extra/test/clang-tidy/zircon-fbl-limits.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-fbl-limits.cpp
@@ -0,0 +1,51 @@
+// RUN: %check_clang_tidy %s zircon-fbl-limits %t -- -- -isystem %S/Inputs/zircon
+
+#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including fbl/limits.h is deprecated
+// CHECK-FIXES-NOT: #include 
+// CHECK-FIXES: #include 
+
+namespace fbl {
+
+template 
+class numeric_limits {};
+
+#define SPECIALIZE_INT_FBL(type) \
+  template <>\
+  class numeric_limits {};
+
+SPECIALIZE_INT_FBL(short)
+
+numeric_limits lim;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of fbl::numeric_limits is deprecated, use std::numeric_limits instead
+// CHECK-FIXES: std::numeric_limits lim;
+
+} // namespace fbl
+
+namespace std {
+
+template 
+class numeric_limits {};
+
+#define SPECIALIZE_INT_STD(type) \
+  template <>\
+  class numeric_limits {};
+
+SPECIALIZE_INT_STD(short)
+
+numeric_limits lim;
+
+} // namespace std
+
+#define DECLARE() fbl::numeric_limits macrolim
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use of fbl::numeric_limits is deprecated, use std::numeric_limits instead
+// CHECK-FIXES: #define DECLARE() std::numeric_limits macrolim
+
+int main() {
+  fbl::numeric_limits fbllim;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of fbl::numeric_limits is deprecated, use std::numeric_limits instead
+  // CHECK-FIXES: std::numeric_limits fbllim;
+  std::numeric_limits stdlim;
+
+  DECLARE();
+}
Index: clang-tools-extra/test/clang-tidy/zircon-fbl-limits-transitive.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-fbl-limits-transitive.cpp
@@ -0,0 +1,11 @@
+// RUN: rm -rf %T/Headers
+// RUN: mkdir %T/Headers
+// RUN: cp -r %S/Inputs/zircon %T/Headers/zircon
+// RUN: %check_clang_tidy %s zircon-fbl-limits %t -- -header-filter=.* -- -std=c++11 -I %T/Headers/zircon
+// RUN: FileCheck -input-file=%T/Headers/zircon/transitive.h %s -check-prefix=CHECK-FIXES
+// RUN: rm -rf %T/Headers
+
+// transitive.h includes 
+#include "transitive.h"
+// CHECK-NOTES: :2:1: warning: including fbl/limits.h is deprecated, transitively included from {{.*}}.
+// CHECK-FIXES-NOT: #include 
Index: clang-tools-extra/test/clang-tidy/Inputs/zircon/transitive.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/Inputs/zircon/transitive.h
@@ -0,0 +1,2 @@
+// Transitively included headers.
+#include 
Index: clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-limits.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-limits.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - zircon-fbl-limits
+
+zircon-fbl-limits
+=
+
+This check is part of the migration checks for moving Zircon user code to use
+the C++ standard library.
+
+It suggests converting uses of ``fbl::numeric_limits`` to
+``std::numeric_limits``, and suggests inserting the  header. It
+also suggests the removal of all uses of the  header, as all
+declarations therein will be replaced by the check and the appropriate
+ replacement header recommended.
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
@@ -254,4 +254,5 @@
readability-string-compare
readability-uniqueptr-delete-release
readability-uppercase-literal-suffix
+   zircon-fbl-limits
zircon-temporary-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -182,6 +182,15 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- New :doc:`zircon-fbl-limits
+  ` check.
+
+  Suggests converting uses of ``fbl::numeric_limits`` to
+  ``std::numeric_limits``, and suggests 

[PATCH] D54168: [clang-tidy] Zircon fbl::move -> std::move

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 172889.
juliehockett marked 4 inline comments as done.

https://reviews.llvm.org/D54168

Files:
  clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
  clang-tools-extra/clang-tidy/zircon/FblMoveCheck.cpp
  clang-tools-extra/clang-tidy/zircon/FblMoveCheck.h
  clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-move.rst
  clang-tools-extra/test/clang-tidy/zircon-fbl-move.cpp

Index: clang-tools-extra/test/clang-tidy/zircon-fbl-move.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-fbl-move.cpp
@@ -0,0 +1,37 @@
+// RUN: %check_clang_tidy %s zircon-fbl-move %t
+
+// CHECK-FIXES: #include 
+
+namespace fbl {
+
+void move(int i) {}
+
+} // namespace fbl
+
+namespace std {
+
+void move(int i) {}
+
+} // namespace std
+
+#define MOVE(i) fbl::move(i)
+// CHECK-NOTES: [[@LINE-1]]:17: warning: fbl::move is deprecated, use std::move instead
+// CHECK-FIXES: #define MOVE(i) std::move(i)
+
+int main() {
+  int i = 0;
+  fbl::move(i);
+  // CHECK-NOTES: [[@LINE-1]]:3: warning: fbl::move is deprecated, use std::move instead
+  // CHECK-FIXES: std::move(i)
+
+  int j = 0;
+  std::move(j);
+
+  void (*foo)(int);
+  foo = ::move;
+  // CHECK-NOTES: [[@LINE-1]]:10: warning: fbl::move is deprecated, use std::move instead
+  // CHECK-FIXES: foo = ::move
+
+  int k = 0;
+  MOVE(k);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-move.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-move.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - zircon-fbl-move
+
+zircon-fbl-move
+===
+
+This check is part of the migration checks for moving Zircon user code to use
+the C++ standard library.
+
+It suggests converting uses of ``fbl::move`` to ``std::move``, and suggests
+inserting the  header.
+
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
@@ -254,4 +254,5 @@
readability-string-compare
readability-uniqueptr-delete-release
readability-uppercase-literal-suffix
+   zircon-fbl-move
zircon-temporary-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -182,6 +182,12 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- New :doc:`zircon-fbl-move
+  ` check.
+
+  Suggests converting uses of ``fbl::move`` to
+  ``std::move``, and suggests inserting the  header.
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
+++ clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "FblMoveCheck.h"
 #include "TemporaryObjectsCheck.h"
 
 using namespace clang::ast_matchers;
@@ -22,6 +23,8 @@
 class ZirconModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"zircon-fbl-move");
 CheckFactories.registerCheck(
 "zircon-temporary-objects");
   }
Index: clang-tools-extra/clang-tidy/zircon/FblMoveCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/zircon/FblMoveCheck.h
@@ -0,0 +1,45 @@
+//===--- FblMoveCheck.h - clang-tidy *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ZIRCON_FBLINCLUDENEWCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ZIRCON_FBLINCLUDENEWCHECK_H
+
+#include "../ClangTidy.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang {
+namespace tidy {
+namespace zircon {
+
+/// Replace instances of `fbl::move` with `std::move`, and adds the
+/// `` header if a replacement occurs.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/zircon-fbl-move.html
+class FblMoveCheck : public ClangTidyCheck {
+public:
+  FblMoveCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context),
+

[PATCH] D54168: [clang-tidy] Zircon fbl::move -> std::move

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

In https://reviews.llvm.org/D54168#1289128, @JonasToth wrote:

> I think this check is ok in the current form, but does it really need to be 
> in upstream clang-tidy? You said its for migration only, so it is not 
> valuable for you for a long time either?


So yes, this check is for a migration, and we would delete it once regressions 
weren't possible. We would like the suite to be in upstream, however, because 
we use the ToT llvm/clang/tools/etc, and don't want to have to fork just to use 
clang-tidy for this sort of thing. Since clang-tidy doesn't provide any way to 
have external checks to the tool itself, upstreaming is the most ideal option.

Orthogonal to our particular build setup, it'd also be nice to have an example 
of this sort of migration done by clang-tidy in-tree. There has been a lot of 
discussion recently about doing migrations with clang-tidy, but it's always 
describing an internal migration that uses a forked tree and a private suite of 
checks that can't be released.


https://reviews.llvm.org/D54168



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

[PATCH] D54169: [clang-tidy] Zircon ->

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: aaron.ballman, alexfh, hokein.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.

Adds a check to convert  to std .

This check is part of a set of migration checks as we prepare to move Zircon 
user code to use the C++ standard library, and should prevent regressions after 
the migration is complete.


https://reviews.llvm.org/D54169

Files:
  clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
  clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp
  clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.h
  clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-limits.rst
  clang-tools-extra/test/clang-tidy/Inputs/zircon/fbl/limits.h
  clang-tools-extra/test/clang-tidy/Inputs/zircon/transitive.h
  clang-tools-extra/test/clang-tidy/zircon-fbl-limits-transitive.cpp
  clang-tools-extra/test/clang-tidy/zircon-fbl-limits.cpp

Index: clang-tools-extra/test/clang-tidy/zircon-fbl-limits.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-fbl-limits.cpp
@@ -0,0 +1,45 @@
+// RUN: %check_clang_tidy %s zircon-fbl-limits %t -- -- -isystem %S/Inputs/zircon
+
+#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including fbl/limits.h is deprecated
+// CHECK-FIXES-NOT: #include 
+// CHECK-FIXES: #include 
+
+namespace fbl {
+
+template 
+class numeric_limits {};
+
+#define SPECIALIZE_INT_FBL(type) \
+  template <>\
+  class numeric_limits {};
+
+SPECIALIZE_INT_FBL(short)
+
+numeric_limits lim;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of fbl::numeric_limits is deprecated, use std::numeric_limits instead
+// CHECK-FIXES: std::numeric_limits lim;
+
+} // namespace fbl
+
+namespace std {
+
+template 
+class numeric_limits {};
+
+#define SPECIALIZE_INT_STD(type) \
+  template <>\
+  class numeric_limits {};
+
+SPECIALIZE_INT_STD(short)
+
+numeric_limits lim;
+
+} // namespace std
+
+int main() {
+  fbl::numeric_limits fbllim;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of fbl::numeric_limits is deprecated, use std::numeric_limits instead
+  // CHECK-FIXES: std::numeric_limits fbllim;
+  std::numeric_limits stdlim;
+}
Index: clang-tools-extra/test/clang-tidy/zircon-fbl-limits-transitive.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-fbl-limits-transitive.cpp
@@ -0,0 +1,11 @@
+// RUN: rm -rf %T/Headers
+// RUN: mkdir %T/Headers
+// RUN: cp -r %S/Inputs/zircon %T/Headers/zircon
+// RUN: %check_clang_tidy %s zircon-fbl-limits %t -- -header-filter=.* -- -std=c++11 -I %T/Headers/zircon
+// RUN: FileCheck -input-file=%T/Headers/zircon/transitive.h %s -check-prefix=CHECK-FIXES
+// RUN: rm -rf %T/Headers
+
+// transitive.h includes 
+#include "transitive.h"
+// CHECK-NOTES: :2:1: warning: including fbl/limits.h is deprecated, transitively included from {{.*}}.
+// CHECK-FIXES-NOT: #include 
Index: clang-tools-extra/test/clang-tidy/Inputs/zircon/transitive.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/Inputs/zircon/transitive.h
@@ -0,0 +1,2 @@
+// Transitively included headers.
+#include 
Index: clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-limits.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-limits.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - zircon-fbl-limits
+
+zircon-fbl-limits
+=
+
+This check is part of the migration checks for moving Zircon user code to use
+the C++ standard library.
+
+It suggests converting uses of ``fbl::numeric_limits`` to
+``std::numeric_limits``, and suggests inserting the  header. It
+also suggests the removal of all uses of the  header, as all
+declarations therein will be replaced by the check and the appropriate
+ replacement header recommended.
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
@@ -254,4 +254,5 @@
readability-string-compare
readability-uniqueptr-delete-release
readability-uppercase-literal-suffix
+   zircon-fbl-limits
zircon-temporary-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -182,6 +182,13 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- New :doc:`zircon-fbl-limits
+  ` check.
+
+  This check is part of the 

[PATCH] D54168: [clang-tidy] Zircon fbl::move -> std::move

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: aaron.ballman, alexfh, hokein.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.

Adds a check to convert fbl::move to std::move.

This check is part of a set of migration checks as we prepare to move Zircon 
user code to use the C++ standard library, and should prevent regressions after 
the migration is complete.


https://reviews.llvm.org/D54168

Files:
  clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
  clang-tools-extra/clang-tidy/zircon/FblMoveCheck.cpp
  clang-tools-extra/clang-tidy/zircon/FblMoveCheck.h
  clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-move.rst
  clang-tools-extra/test/clang-tidy/zircon-fbl-move.cpp

Index: clang-tools-extra/test/clang-tidy/zircon-fbl-move.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-fbl-move.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s zircon-fbl-move %t
+
+namespace fbl {
+
+void move(int i) {}
+
+} // namespace fbl
+
+namespace std {
+
+void move(int i) {}
+
+} // namespace std
+
+int main() {
+  int i;
+  fbl::move(i);
+  // CHECK-NOTES: [[@LINE-1]]:3: warning: fbl::move is deprecated, use std::move instead
+  // CHECK-FIXES: #include 
+  // CHECK-FIXES: std::move
+
+  std::move(i);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-move.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-move.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - zircon-fbl-move
+
+zircon-fbl-move
+===
+
+This check is part of the migration checks for moving Zircon user code to use
+the C++ standard library.
+
+It suggests converting uses of ``fbl::move`` to ``std::move``, and suggests
+inserting the  header.
+
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
@@ -254,4 +254,5 @@
readability-string-compare
readability-uniqueptr-delete-release
readability-uppercase-literal-suffix
+   zircon-fbl-move
zircon-temporary-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -182,6 +182,13 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- New :doc:`zircon-fbl-move
+  ` check.
+
+  This check is part of the migration checks for moving Zircon user code to use
+  the C++ standard library. It suggests converting uses of ``fbl::move`` to
+  ``std::move``, and suggests inserting the  header.
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
+++ clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "FblMoveCheck.h"
 #include "TemporaryObjectsCheck.h"
 
 using namespace clang::ast_matchers;
@@ -22,6 +23,8 @@
 class ZirconModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"zircon-fbl-move");
 CheckFactories.registerCheck(
 "zircon-temporary-objects");
   }
Index: clang-tools-extra/clang-tidy/zircon/FblMoveCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/zircon/FblMoveCheck.h
@@ -0,0 +1,45 @@
+//===--- FblMoveCheck.h - clang-tidy *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ZIRCON_FBLINCLUDENEWCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ZIRCON_FBLINCLUDENEWCHECK_H
+
+#include "../ClangTidy.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang {
+namespace tidy {
+namespace zircon {
+
+/// Replace instances of fbl::move with std::move, and adds the  header
+/// if a replacement occurs.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/zircon-fbl-move.html
+class FblMoveCheck : public ClangTidyCheck {
+public:
+  FblMoveCheck(StringRef Name, ClangTidyContext 

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

[Julie Hockett via Phabricator via cfe-commits]

juliehockett planned changes to this revision.
juliehockett marked 5 inline comments as done.
juliehockett added a comment.

In https://reviews.llvm.org/D53882#1282219, @aaron.ballman wrote:

> I am a bit confused by what this check is trying to accomplish. It seems like 
> this is intended to catch use of anything declared within the standard 
> library, but that includes library support things that are needed to write 
> useful code. For instance, it seems this means users cannot use initializer 
> lists or type traits. I'm not even certain users can overload `operator 
> new()` because that uses `std::size_t` as an argument.
>
> Can you expound on the requirements in a bit more detail? Is it truly "no 
> standard library functionality at all, including things required to be 
> supported in freestanding implementations"?


Yes. We're in the process of enabling the C++ standard library in the Zircon 
userspace code, and there is a firm requirement that no kernel code use 
anything in the std namespace (which declares its own implementations of things 
like type traits).

That said, the policy rollout has shifted a bit since I implemented this and so 
I'll be reworking it a bit (and also likely renaming it to 
zircon-kernel-no-std-namespace, so we can disable it for userspace code). 
Thanks all for your comments!


https://reviews.llvm.org/D53882



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

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 171810.
juliehockett marked 5 inline comments as done.

https://reviews.llvm.org/D53882

Files:
  clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
  clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp
  clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.h
  clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst
  clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp

Index: clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s zircon-no-std-namespace %t
+
+int func() { return 1; }
+
+namespace std {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+typedef int std_int;
+int std_func() { return 1; }
+
+int a = 1;
+int b = func();
+std_int c;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+int d = std_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+}
+
+// Warn on uses of quailfied std namespace.
+int i = 1;
+int j = func();
+std::std_int k;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+int l = std::std_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+
+namespace foo {
+
+int w = 1;
+int x = func();
+std::std_int y;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+int z = std::std_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+}
+
+// Warn on the alias declaration, and on users of it.
+namespace bar = std;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+bar::std_int q;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+// Warn on the using declaration, and on users of it.
+using namespace std;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+std_int r = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
Index: clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst
@@ -0,0 +1,30 @@
+.. title:: clang-tidy - zircon-no-std-namespace
+
+zircon-no-std-namespace
+===
+
+Ensures code does not use ``namespace std`` as that violates Zircon's
+kernel libc++ policy. 
+
+Any code that uses:
+
+.. code-block:: c++
+
+ namespace std {
+  ...
+ }
+
+or 
+
+.. code-block:: c++
+
+ using namespace std;
+
+or uses a declaration or function from the ``std`` namespace:
+
+.. code-block:: c++
+
+ std::string foo;
+ std::move(foo);
+
+will be prompted with a warning.
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
@@ -252,4 +252,5 @@
readability-string-compare
readability-uniqueptr-delete-release
readability-uppercase-literal-suffix
+   zircon-no-std-namespace
zircon-temporary-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -170,6 +170,12 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- New :doc:`zircon-no-std-namespace
+  ` check.
+
+  Warns when the `std` namespace is used, as its use is against Zircon's libc++
+  policy for the kernel.
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
+++ clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "NoStdNamespaceCheck.h"
 #include "TemporaryObjectsCheck.h"
 
 using namespace clang::ast_matchers;
@@ -22,6 +23,8 @@
 class ZirconModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:71
+void NoStdNamespaceCheck::check(const MatchFinder::MatchResult ) {
+  if (const auto *D = Result.Nodes.getNodeAs("stdVar"))
+diag(D->getBeginLoc(),

JonasToth wrote:
> Please create a `StringRef` for the diagnostic message and reuse that.
> 
> Did you consider merging all `Decl` classes if you just use 
> `getNodeAs("common_decl_name")`?
Yes, but getting the location right posed an issue there. The `std` token is 
not always at `D->getLocation()`.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst:1
+.. title:: clang-tidy - zircon-no-std-namespace
+

JonasToth wrote:
> Could you please clarify what exactly is forbidden?
> 
> __Using__ stuff from `std` like `std::string` in normal "user" code or 
> __opening__ `std` to add stuff?
> 
> If the first thing is the case why are the variables flagged but not the 
> functions? And adding things to namespace `std` is AFAIK UB with the 
> exception of template-function specializations, so maybe that would be worth 
> a general check?
Both. Documentation updated -- the uses in particular are to be flagged.



Comment at: clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp:33
+int x = func();
+std::std_int y;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not 
allowed in Zircon kernel code

JonasToth wrote:
> What about using `int64_t` and these typedefs. Are they forbidden, too?
Yes, which is why that's tested. Is there an additional test case I'm missing 
regarding those?


https://reviews.llvm.org/D53882



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

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: aaron.ballman, hokein, ilya-biryukov.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.

Adds a checker to warn against using the std namespace, as Zircon's kernel 
lib++ policy does not allow it. Written documentation of the policy is not yet 
published, I will add the link when it is.


https://reviews.llvm.org/D53882

Files:
  clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
  clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp
  clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.h
  clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst
  clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp

Index: clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s zircon-no-std-namespace %t
+
+int func() { return 1; }
+
+namespace std {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+typedef int std_int;
+int std_func() { return 1; }
+
+int a = 1;
+int b = func();
+std_int c;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+int d = std_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+}
+
+// Warn on uses of quailfied std namespace.
+int i = 1;
+int j = func();
+std::std_int k;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+int l = std::std_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+
+namespace foo {
+
+int w = 1;
+int x = func();
+std::std_int y;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+int z = std::std_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+}
+
+// Warn on the alias declaration, and on users of it.
+namespace bar = std;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+bar::std_int q;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+// Warn on the using declaration, and on users of it.
+using namespace std;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+std_int r = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
Index: clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - zircon-no-std-namespace
+
+zircon-no-std-namespace
+===
+
+Ensures code does not open ``namespace std`` as that violates Zircon's
+kernel libc++ policy. 
+
+Any code that uses:
+
+.. code-block:: c++
+
+ namespace std {
+  ...
+ }
+
+or 
+
+.. code-block:: c++
+
+ using namespace std;
+
+or uses a declaration from the ``std`` namespace:
+
+.. code-block:: c++
+
+ std::string foo;
+
+will be prompted with a warning.
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
@@ -252,4 +252,5 @@
readability-string-compare
readability-uniqueptr-delete-release
readability-uppercase-literal-suffix
+   zircon-no-std-namespace
zircon-temporary-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -170,6 +170,12 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- New :doc:`zircon-no-std-namespace
+  ` check.
+
+  Warns when the `std` namespace is used, as its use is against Zircon's libc++
+  policy for the kernel.
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
+++ clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include 

[PATCH] D53170: [clang-doc] Switch to default to all-TUs executor

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE345418: [clang-doc] Switch to default to all-TUs executor 
(authored by juliehockett, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53170?vs=169414=171331#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53170

Files:
  clang-doc/tool/ClangDocMain.cpp
  test/clang-doc/single-file-public.cpp
  test/clang-doc/single-file.cpp


Index: clang-doc/tool/ClangDocMain.cpp
===
--- clang-doc/tool/ClangDocMain.cpp
+++ clang-doc/tool/ClangDocMain.cpp
@@ -31,7 +31,6 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
-#include "clang/Tooling/StandaloneExecution.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/Support/CommandLine.h"
@@ -169,6 +168,7 @@
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
 
+  ExecutorName.setInitialValue("all-TUs");
   auto Exec = clang::tooling::createExecutorFromCommandLineArgs(
   argc, argv, ClangDocCategory);
 
Index: test/clang-doc/single-file.cpp
===
--- test/clang-doc/single-file.cpp
+++ test/clang-doc/single-file.cpp
@@ -2,7 +2,7 @@
 // RUN: mkdir %t
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
-// RUN: clang-doc --doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: clang-doc --doxygen --executor=standalone -p %t %t/test.cpp 
-output=%t/docs
 // RUN: cat %t/docs/GlobalNamespace.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
Index: test/clang-doc/single-file-public.cpp
===
--- test/clang-doc/single-file-public.cpp
+++ test/clang-doc/single-file-public.cpp
@@ -2,7 +2,7 @@
 // RUN: mkdir %t
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
-// RUN: clang-doc --doxygen --public -p %t %t/test.cpp -output=%t/docs
+// RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp 
-output=%t/docs
 // RUN: cat %t/docs/Record.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 


Index: clang-doc/tool/ClangDocMain.cpp
===
--- clang-doc/tool/ClangDocMain.cpp
+++ clang-doc/tool/ClangDocMain.cpp
@@ -31,7 +31,6 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
-#include "clang/Tooling/StandaloneExecution.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/Support/CommandLine.h"
@@ -169,6 +168,7 @@
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
 
+  ExecutorName.setInitialValue("all-TUs");
   auto Exec = clang::tooling::createExecutorFromCommandLineArgs(
   argc, argv, ClangDocCategory);
 
Index: test/clang-doc/single-file.cpp
===
--- test/clang-doc/single-file.cpp
+++ test/clang-doc/single-file.cpp
@@ -2,7 +2,7 @@
 // RUN: mkdir %t
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
-// RUN: clang-doc --doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: clang-doc --doxygen --executor=standalone -p %t %t/test.cpp -output=%t/docs
 // RUN: cat %t/docs/GlobalNamespace.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
Index: test/clang-doc/single-file-public.cpp
===
--- test/clang-doc/single-file-public.cpp
+++ test/clang-doc/single-file-public.cpp
@@ -2,7 +2,7 @@
 // RUN: mkdir %t
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
-// RUN: clang-doc --doxygen --public -p %t %t/test.cpp -output=%t/docs
+// RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp -output=%t/docs
 // RUN: cat %t/docs/Record.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits