[clang-tools-extra] [clang-doc] add support for block commands in clang-doc html output (PR #101108)
@@ -631,7 +631,19 @@ static std::unique_ptr genHTML(const CommentInfo
) {
return nullptr;
return std::move(ParagraphComment);
}
-
ilovepi wrote:
Why remove the new line?
https://github.com/llvm/llvm-project/pull/101108
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add support for block commands in clang-doc html output (PR #101108)
@@ -619,7 +620,6 @@ static std::unique_ptr genHTML(const CommentInfo
) {
}
return std::move(FullComment);
}
-
ilovepi wrote:
Unrelated?
https://github.com/llvm/llvm-project/pull/101108
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add support for block commands in clang-doc html output (PR #101108)
@@ -197,7 +197,9 @@ TEST(HTMLGeneratorTest, emitRecordHTML) {
Members
- private int X
+
+private int X
ilovepi wrote:
Agreed.
https://github.com/llvm/llvm-project/pull/101108
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add support for block commands in clang-doc html output (PR #101108)
@@ -418,9 +419,13 @@ genRecordMembersBlock(const
llvm::SmallVector ,
if (Access != "")
Access = Access + " ";
auto LIBody = std::make_unique(HTMLTag::TAG_LI);
-LIBody->Children.emplace_back(std::make_unique(Access));
-LIBody->Children.emplace_back(genReference(M.Type, ParentInfoDir));
-LIBody->Children.emplace_back(std::make_unique(" " + M.Name));
+auto MemberDecl = std::make_unique(HTMLTag::TAG_DIV);
+MemberDecl->Children.emplace_back(std::make_unique(Access));
+MemberDecl->Children.emplace_back(genReference(M.Type, ParentInfoDir));
+MemberDecl->Children.emplace_back(std::make_unique(" " +
M.Name));
+if (!M.Description.empty())
+ LIBody->Children.emplace_back(genHTML(M.Description));
+LIBody->Children.emplace_back(std::move(MemberDecl));
ilovepi wrote:
Yeah, lets split this up a bit. I'll leave the ordering up to you, but the
other feature seems to be roughly independent.
https://github.com/llvm/llvm-project/pull/101108
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add support for block commands in clang-doc html output (PR #101108)
@@ -197,7 +197,9 @@ TEST(HTMLGeneratorTest, emitRecordHTML) {
Members
- private int X
+
+private int X
ilovepi wrote:
Should these be `private int X`, like this or should we have separate lists for
`public`, `protected` and `private`?
https://github.com/llvm/llvm-project/pull/101108
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add support for block commands in clang-doc html output (PR #101108)
@@ -418,9 +419,13 @@ genRecordMembersBlock(const
llvm::SmallVector ,
if (Access != "")
Access = Access + " ";
auto LIBody = std::make_unique(HTMLTag::TAG_LI);
-LIBody->Children.emplace_back(std::make_unique(Access));
-LIBody->Children.emplace_back(genReference(M.Type, ParentInfoDir));
-LIBody->Children.emplace_back(std::make_unique(" " + M.Name));
+auto MemberDecl = std::make_unique(HTMLTag::TAG_DIV);
+MemberDecl->Children.emplace_back(std::make_unique(Access));
+MemberDecl->Children.emplace_back(genReference(M.Type, ParentInfoDir));
+MemberDecl->Children.emplace_back(std::make_unique(" " +
M.Name));
+if (!M.Description.empty())
+ LIBody->Children.emplace_back(genHTML(M.Description));
+LIBody->Children.emplace_back(std::move(MemberDecl));
ilovepi wrote:
Is this required for Block commands? This seems like a separate change...
https://github.com/llvm/llvm-project/pull/101108
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CMake][Fuchsia] Include libunwind and libc++abi in baremetal build (PR #100908)
https://github.com/ilovepi approved this pull request. https://github.com/llvm/llvm-project/pull/100908 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] switched from using relative to absolute paths (PR #93281)
@@ -0,0 +1,7 @@ +// RUN: rm -rf %t && mkdir %t +// RUN: cp "%s" "%t/test.cpp" ilovepi wrote: Looks like I missed this. Please don’t copy the test file around. To be blunt this pattern has been brought up in multiple PRs and even in previous incarnations of this one. Use the same conventions we’ve insisted on in other tests here. https://github.com/llvm/llvm-project/pull/93281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] switched from using relative to absolute paths (PR #93281)
ilovepi wrote: Hmm, seems like tests are failing. PTAL. https://github.com/llvm/llvm-project/pull/93281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] switched from using relative to absolute paths (PR #93281)
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --format=html --executor=standalone -p %s --output=%t
+// RUN: FileCheck %s -input-file=%t/index_json.js -check-prefix=JSON-INDEX
+// RUN: rm -rf %t
+
+// JSON-INDEX: var RootPath = "{{.*}}";
ilovepi wrote:
Is there a way to make this check more meaningful? Can we match nothing in the
path itself?
https://github.com/llvm/llvm-project/pull/93281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] switched from using relative to absolute paths (PR #93281)
https://github.com/ilovepi approved this pull request. LGTM, but can we improve the check for RootPath somehow? I feel like there should be something we can match there. https://github.com/llvm/llvm-project/pull/93281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] switched from using relative to absolute paths (PR #93281)
https://github.com/ilovepi edited https://github.com/llvm/llvm-project/pull/93281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
ilovepi wrote: Oh, can you also update the title and commit message? You're not really adding a short circuit, you're just memoizing visited items, and avoiding reprocessing them. A bit more context, and a summary of your findings on the changes to output would also be good to have in the commit log. https://github.com/llvm/llvm-project/pull/96809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
ilovepi wrote: Seems to be a formatting issue. `git clang-format` should fix it. https://github.com/llvm/llvm-project/pull/96809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
@@ -243,6 +243,7 @@ Example usage for a project using a compile commands database: // Fail early if an invalid format was provided. std::string Format = getFormatString(); + llvm::outs() << "Unoptimized\n"; ilovepi wrote: nit: leftover from debugging? https://github.com/llvm/llvm-project/pull/96809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
https://github.com/ilovepi edited https://github.com/llvm/llvm-project/pull/96809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
https://github.com/ilovepi approved this pull request. If we can confirm that the only differences in documentation generation are due to ordering, then I think it makes sense to land this patch. https://github.com/llvm/llvm-project/pull/96809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ASTContext] Add a break to nested switch in `encodeTypeForFunctionPointerAuth` (PR #99763)
@@ -3363,6 +3363,7 @@ static void encodeTypeForFunctionPointerAuth(const
ASTContext ,
#include "clang/Basic/RISCVVTypes.def"
llvm_unreachable("not yet implemented");
}
+break;
ilovepi wrote:
won't you get an error for having a `default` case on a fully covered switch?
Plus that will remain working if someone adds a new enum value, but doesn't
update this code, right?
https://github.com/llvm/llvm-project/pull/99763
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ASTContext] Add a break to nested switch in `encodeTypeForFunctionPointerAuth` (PR #99763)
ilovepi wrote: Ah, it looks like we had another set of these. I just filed https://github.com/llvm/llvm-project/issues/99901 after a similar issue was reported in LLD. https://github.com/llvm/llvm-project/pull/99763 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
ilovepi wrote: > When sorted by USR, there are no differences between the index between this > patch and what's on main, I'm not sure I completely follow. are you saying that when you sort the contents of all the files you no longer have any differences? So you're sure the only differences in output are due to ordering? I'd suppose that is rather expected from a racy framework, like clang-doc. Once the tests PRs are landed, can you rebase this patch, so we can see the affects on the larger tests? https://github.com/llvm/llvm-project/pull/96809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
https://github.com/ilovepi approved this pull request. LGTM modulo a small nit in the naming, we can always add more instrumentation if we need it, but this is a good start. https://github.com/llvm/llvm-project/pull/97644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
https://github.com/ilovepi edited https://github.com/llvm/llvm-project/pull/97644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -670,6 +671,7 @@ llvm::Error ClangDocBitcodeReader::readRecord(unsigned ID,
T I) {
template <>
llvm::Error ClangDocBitcodeReader::readRecord(unsigned ID, Reference *I) {
+ llvm::TimeTraceScope("Reducing infos", "readRecord");
ilovepi wrote:
I know this is terminology already in clang-doc, but lets come up w/ something
a bit more informative than "Reducing Infos". TBH, the whole "Infos" bit has
always rubbed me the wrong way, so even just `Reducing Results` is better IMO.
Use your judgement, but I think we can find a better name for these than
`Infos`.
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CMake][Fuchsia] Build libc++ on top libc for baremetal (PR #99009)
https://github.com/ilovepi approved this pull request. Not a fan of this approach, but, seeing as it’s temporary and only affects our builds, I suppose it’s fine for now. https://github.com/llvm/llvm-project/pull/99009 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
https://github.com/ilovepi approved this pull request. LGTM, once the LINE checks are in a more stable position w/ their own prefixes, and a couple of nits on whitespace. https://github.com/llvm/llvm-project/pull/97681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
@@ -0,0 +1,274 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --format=html --output=%t --executor=standalone %s
+// RUN: clang-doc --format=md --output=%t --executor=standalone %s
+// RUN: FileCheck %s < %t/index_json.js -check-prefix=JSON-INDEX
+// RUN: FileCheck %s < %t/@nonymous_namespace/AnonClass.html
-check-prefix=HTML-ANON-CLASS
+// RUN: FileCheck %s < %t/@nonymous_namespace/index.html
-check-prefix=HTML-ANON-INDEX
+// RUN: FileCheck %s < %t/AnotherNamespace/ClassInAnotherNamespace.html
-check-prefix=HTML-ANOTHER-CLASS
+// RUN: FileCheck %s < %t/AnotherNamespace/index.html
-check-prefix=HTML-ANOTHER-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/index.html
-check-prefix=HTML-GLOBAL-INDEX
+// RUN: FileCheck %s <
%t/PrimaryNamespace/NestedNamespace/ClassInNestedNamespace.html
-check-prefix=HTML-NESTED-CLASS
+// RUN: FileCheck %s < %t/PrimaryNamespace/NestedNamespace/index.html
-check-prefix=HTML-NESTED-INDEX
+// RUN: FileCheck %s < %t/PrimaryNamespace/index.html
-check-prefix=HTML-PRIMARY-INDEX
+// RUN: FileCheck %s < %t/PrimaryNamespace/ClassInPrimaryNamespace.html
-check-prefix=HTML-PRIMARY-CLASS
+// RUN: FileCheck %s < %t/@nonymous_namespace/AnonClass.md
-check-prefix=MD-ANON-CLASS
+// RUN: FileCheck %s < %t/@nonymous_namespace/index.md
-check-prefix=MD-ANON-INDEX
+// RUN: FileCheck %s < %t/AnotherNamespace/ClassInAnotherNamespace.md
-check-prefix=MD-ANOTHER-CLASS
+// RUN: FileCheck %s < %t/AnotherNamespace/index.md
-check-prefix=MD-ANOTHER-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/index.md
-check-prefix=MD-GLOBAL-INDEX
+// RUN: FileCheck %s <
%t/PrimaryNamespace/NestedNamespace/ClassInNestedNamespace.md
-check-prefix=MD-NESTED-CLASS
+// RUN: FileCheck %s < %t/PrimaryNamespace/NestedNamespace/index.md
-check-prefix=MD-NESTED-INDEX
+// RUN: FileCheck %s < %t/PrimaryNamespace/index.md
-check-prefix=MD-PRIMARY-INDEX
+// RUN: FileCheck %s < %t/PrimaryNamespace/ClassInPrimaryNamespace.md
-check-prefix=MD-PRIMARY-CLASS
+// RUN: FileCheck %s < %t/all_files.md -check-prefix=MD-ALL-FILES
+// RUN: FileCheck %s < %t/index.md -check-prefix=MD-INDEX
+
+// MD-ANON-INDEX: # namespace @nonymous_namespace
+// MD-ANON-INDEX: Anonymous Namespace
+// MD-ANON-INDEX: ## Records
+// MD-ANON-INDEX: * [AnonClass](AnonClass.md)
+// MD-ANON-INDEX: ## Functions
+// MD-ANON-INDEX: ### anonFunction
+// MD-ANON-INDEX: *void anonFunction()*
+// MD-ANON-INDEX: *Defined at
{{.*}}clang-tools-extra{{[\/]}}test{{[\/]}}clang-doc{{[\/]}}namespace.cpp#[[@LINE+15]]*
+
+// HTML-ANON-INDEX: namespace @nonymous_namespace
+// HTML-ANON-INDEX: Anonymous Namespace
+// HTML-ANON-INDEX: Records
+// HTML-ANON-INDEX: AnonClass
+// HTML-ANON-INDEX: Functions
+// HTML-ANON-INDEX: anonFunction
+// HTML-ANON-INDEX: void anonFunction()
+// HTML-ANON-INDEX: Defined at line [[@LINE+6]] of file
{{.*}}clang-tools-extra{{[\/]}}test{{[\/]}}clang-doc{{[\/]}}namespace.cpp
+
+// Anonymous Namespace
+namespace
+{
+
ilovepi wrote:
nit: extra newline.
https://github.com/llvm/llvm-project/pull/97681
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
@@ -0,0 +1,274 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --format=html --output=%t --executor=standalone %s
+// RUN: clang-doc --format=md --output=%t --executor=standalone %s
+// RUN: FileCheck %s < %t/index_json.js -check-prefix=JSON-INDEX
+// RUN: FileCheck %s < %t/@nonymous_namespace/AnonClass.html
-check-prefix=HTML-ANON-CLASS
+// RUN: FileCheck %s < %t/@nonymous_namespace/index.html
-check-prefix=HTML-ANON-INDEX
+// RUN: FileCheck %s < %t/AnotherNamespace/ClassInAnotherNamespace.html
-check-prefix=HTML-ANOTHER-CLASS
+// RUN: FileCheck %s < %t/AnotherNamespace/index.html
-check-prefix=HTML-ANOTHER-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/index.html
-check-prefix=HTML-GLOBAL-INDEX
+// RUN: FileCheck %s <
%t/PrimaryNamespace/NestedNamespace/ClassInNestedNamespace.html
-check-prefix=HTML-NESTED-CLASS
+// RUN: FileCheck %s < %t/PrimaryNamespace/NestedNamespace/index.html
-check-prefix=HTML-NESTED-INDEX
+// RUN: FileCheck %s < %t/PrimaryNamespace/index.html
-check-prefix=HTML-PRIMARY-INDEX
+// RUN: FileCheck %s < %t/PrimaryNamespace/ClassInPrimaryNamespace.html
-check-prefix=HTML-PRIMARY-CLASS
+// RUN: FileCheck %s < %t/@nonymous_namespace/AnonClass.md
-check-prefix=MD-ANON-CLASS
+// RUN: FileCheck %s < %t/@nonymous_namespace/index.md
-check-prefix=MD-ANON-INDEX
+// RUN: FileCheck %s < %t/AnotherNamespace/ClassInAnotherNamespace.md
-check-prefix=MD-ANOTHER-CLASS
+// RUN: FileCheck %s < %t/AnotherNamespace/index.md
-check-prefix=MD-ANOTHER-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/index.md
-check-prefix=MD-GLOBAL-INDEX
+// RUN: FileCheck %s <
%t/PrimaryNamespace/NestedNamespace/ClassInNestedNamespace.md
-check-prefix=MD-NESTED-CLASS
+// RUN: FileCheck %s < %t/PrimaryNamespace/NestedNamespace/index.md
-check-prefix=MD-NESTED-INDEX
+// RUN: FileCheck %s < %t/PrimaryNamespace/index.md
-check-prefix=MD-PRIMARY-INDEX
+// RUN: FileCheck %s < %t/PrimaryNamespace/ClassInPrimaryNamespace.md
-check-prefix=MD-PRIMARY-CLASS
+// RUN: FileCheck %s < %t/all_files.md -check-prefix=MD-ALL-FILES
+// RUN: FileCheck %s < %t/index.md -check-prefix=MD-INDEX
+
+// MD-ANON-INDEX: # namespace @nonymous_namespace
+// MD-ANON-INDEX: Anonymous Namespace
+// MD-ANON-INDEX: ## Records
+// MD-ANON-INDEX: * [AnonClass](AnonClass.md)
+// MD-ANON-INDEX: ## Functions
+// MD-ANON-INDEX: ### anonFunction
+// MD-ANON-INDEX: *void anonFunction()*
+// MD-ANON-INDEX: *Defined at
{{.*}}clang-tools-extra{{[\/]}}test{{[\/]}}clang-doc{{[\/]}}namespace.cpp#[[@LINE+15]]*
+
+// HTML-ANON-INDEX: namespace @nonymous_namespace
+// HTML-ANON-INDEX: Anonymous Namespace
+// HTML-ANON-INDEX: Records
+// HTML-ANON-INDEX: AnonClass
+// HTML-ANON-INDEX: Functions
+// HTML-ANON-INDEX: anonFunction
+// HTML-ANON-INDEX: void anonFunction()
+// HTML-ANON-INDEX: Defined at line [[@LINE+6]] of file
{{.*}}clang-tools-extra{{[\/]}}test{{[\/]}}clang-doc{{[\/]}}namespace.cpp
ilovepi wrote:
As mentioned in your other PR, lets check these right after the line they're
checking, and use a `*-LINE` tag to check these w/ `--check-prefixes`. That
should prevent instability in the test due to more test checks getting added.
Once that's done for all these tests, this will probably be good to go, modulo
some nits.
https://github.com/llvm/llvm-project/pull/97681
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
https://github.com/ilovepi edited https://github.com/llvm/llvm-project/pull/97681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add enum test (PR #97679)
@@ -0,0 +1,128 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: clang-doc --format=html --doxygen --output=%t --executor=standalone %s +// RUN: clang-doc --format=md --doxygen --output=%t --executor=standalone %s +// RUN: FileCheck %s < %t/GlobalNamespace/index.html -check-prefix=HTML-INDEX +// RUN: FileCheck %s < %t/GlobalNamespace/Animals.html -check-prefix=HTML-ANIMAL +// RUN: FileCheck %s < %t/Vehicles/index.html -check-prefix=HTML-VEHICLES +// RUN: FileCheck %s < %t/GlobalNamespace/index.md -check-prefix=MD-INDEX +// RUN: FileCheck %s < %t/GlobalNamespace/Animals.md -check-prefix=MD-ANIMAL +// RUN: FileCheck %s < %t/Vehicles/index.md -check-prefix=MD-VEHICLES + + ilovepi wrote: nit: extra newlines https://github.com/llvm/llvm-project/pull/97679 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add enum test (PR #97679)
https://github.com/ilovepi edited https://github.com/llvm/llvm-project/pull/97679 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add enum test (PR #97679)
@@ -0,0 +1,128 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --format=html --doxygen --output=%t --executor=standalone %s
+// RUN: clang-doc --format=md --doxygen --output=%t --executor=standalone %s
+// RUN: FileCheck %s < %t/GlobalNamespace/index.html -check-prefix=HTML-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/Animals.html
-check-prefix=HTML-ANIMAL
+// RUN: FileCheck %s < %t/Vehicles/index.html -check-prefix=HTML-VEHICLES
+// RUN: FileCheck %s < %t/GlobalNamespace/index.md -check-prefix=MD-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/Animals.md -check-prefix=MD-ANIMAL
+// RUN: FileCheck %s < %t/Vehicles/index.md -check-prefix=MD-VEHICLES
+
+
+
+// MD-INDEX: # Global Namespace
+// MD-INDEX: ## Enums
+// MD-INDEX: | enum Color |
+// MD-INDEX: --
+// MD-INDEX: | Red |
+// MD-INDEX: | Green |
+// MD-INDEX: | Blue |
+// MD-INDEX: *Defined at
{{.*}}clang-tools-extra{{[\/]}}test{{[\/]}}clang-doc{{[\/]}}enum.cpp#[[@LINE+13]]*
+// MD-INDEX: **brief** For specifying RGB colors
ilovepi wrote:
This is a lot better, but maybe its better to move this and the HTML line check
below the LINE you want to check and then do
```
enum Color {
// MD-INDEX: *Defined at
{{.*}}clang-tools-extra{{[\/]}}test{{[\/]}}clang-doc{{[\/]}}enum.cpp#[[@LINE-1]]*
// HTML-INDEX: Defined at line [[@LINE-2]] of file
{{.*}}clang-tools-extra{{[\/]}}test{{[\/]}}clang-doc{{[\/]}}enum.cpp
```
Any remaining checks can go below the class, or these line checks could be
MD-INDEX-LINE & HTML-INDEX-LINE, and you can list 2 prefixes to check, then
only the 2 line checks would be out of order w/ the rest. WDYT?
https://github.com/llvm/llvm-project/pull/97679
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add enum test (PR #97679)
https://github.com/ilovepi approved this pull request. LGTM, modulo some nits. I do think you may want to have a separate prefix for the lines, but if you change `--check-prefix=` to `--check-prefixes=HTML-ANIMAL,HTML-ANIMAL-LINE`, you should be able to check them w/o having to rewrite everything. Plus keeping the LINE comments right after the source line makes it a lot easier to keep the tests stable. https://github.com/llvm/llvm-project/pull/97679 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add enum test (PR #97679)
@@ -0,0 +1,121 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --format=html --doxygen --output=%t --executor=standalone %s
+// RUN: clang-doc --format=md --doxygen --output=%t --executor=standalone %s
+// RUN: FileCheck %s < %t/GlobalNamespace/index.html -check-prefix=HTML-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/Animals.html
-check-prefix=HTML-ANIMAL
+// RUN: FileCheck %s < %t/Vehicles/index.html -check-prefix=HTML-VEHICLES
+// RUN: FileCheck %s < %t/GlobalNamespace/index.md -check-prefix=MD-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/Animals.md -check-prefix=MD-ANIMAL
+// RUN: FileCheck %s < %t/Vehicles/index.md -check-prefix=MD-VEHICLES
+
+
+/**
+ * @brief For specifying RGB colors
+ */
+enum Color {
+ Red, // Red
+ Green, // Green
+ Blue // Blue
+};
+
+/**
+ * @brief Shape Types
+ */
+enum Shapes {
ilovepi wrote:
yeah, those cases are distinct, so its good to have both.
https://github.com/llvm/llvm-project/pull/97679
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
ilovepi wrote: To clarify my earlier comment, FileCheck is just matching text on the input based on the check lines. So in this cases the LINE directive should just affect what text will be accepted when doin matching. It won’t try to match the lines to the position in the file being checked. If you run into problems, post the errors here, as we can probably help diagnose them. https://github.com/llvm/llvm-project/pull/97681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
@@ -0,0 +1,270 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --format=html --output=%t --executor=standalone %s
+// RUN: clang-doc --format=md --output=%t --executor=standalone %s
+// RUN: FileCheck %s < %t/index_json.js -check-prefix=JSON-INDEX
+// RUN: FileCheck %s < %t/@nonymous_namespace/AnonClass.html
-check-prefix=HTML-ANON-CLASS
+// RUN: FileCheck %s < %t/@nonymous_namespace/index.html
-check-prefix=HTML-ANON-INDEX
+// RUN: FileCheck %s < %t/AnotherNamespace/ClassInAnotherNamespace.html
-check-prefix=HTML-ANOTHER-CLASS
+// RUN: FileCheck %s < %t/AnotherNamespace/index.html
-check-prefix=HTML-ANOTHER-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/index.html
-check-prefix=HTML-GLOBAL-INDEX
+// RUN: FileCheck %s <
%t/PrimaryNamespace/NestedNamespace/ClassInNestedNamespace.html
-check-prefix=HTML-NESTED-CLASS
+// RUN: FileCheck %s < %t/PrimaryNamespace/NestedNamespace/index.html
-check-prefix=HTML-NESTED-INDEX
+// RUN: FileCheck %s < %t/PrimaryNamespace/index.html
-check-prefix=HTML-PRIMARY-INDEX
+// RUN: FileCheck %s < %t/PrimaryNamespace/ClassInPrimaryNamespace.html
-check-prefix=HTML-PRIMARY-CLASS
+// RUN: FileCheck %s < %t/@nonymous_namespace/AnonClass.md
-check-prefix=MD-ANON-CLASS
+// RUN: FileCheck %s < %t/@nonymous_namespace/index.md
-check-prefix=MD-ANON-INDEX
+// RUN: FileCheck %s < %t/AnotherNamespace/ClassInAnotherNamespace.md
-check-prefix=MD-ANOTHER-CLASS
+// RUN: FileCheck %s < %t/AnotherNamespace/index.md
-check-prefix=MD-ANOTHER-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/index.md
-check-prefix=MD-GLOBAL-INDEX
+// RUN: FileCheck %s <
%t/PrimaryNamespace/NestedNamespace/ClassInNestedNamespace.md
-check-prefix=MD-NESTED-CLASS
+// RUN: FileCheck %s < %t/PrimaryNamespace/NestedNamespace/index.md
-check-prefix=MD-NESTED-INDEX
+// RUN: FileCheck %s < %t/PrimaryNamespace/index.md
-check-prefix=MD-PRIMARY-INDEX
+// RUN: FileCheck %s < %t/PrimaryNamespace/ClassInPrimaryNamespace.md
-check-prefix=MD-PRIMARY-CLASS
+// RUN: FileCheck %s < %t/all_files.md -check-prefix=MD-ALL-FILES
+// RUN: FileCheck %s < %t/index.md -check-prefix=MD-INDEX
+
+// Anonymous Namespace
+namespace
+{
+void anonFunction() {}
+class AnonClass {};
+}
+
+// Primary Namespace
+namespace PrimaryNamespace {
+// Function in PrimaryNamespace
+void functionInPrimaryNamespace() {}
+
+// Class in PrimaryNamespace
+class ClassInPrimaryNamespace {};
+
+// Nested namespace
+namespace NestedNamespace {
+// Function in NestedNamespace
+void functionInNestedNamespace() {}
+// Class in NestedNamespace
+class ClassInNestedNamespace {};
+}
+}
+
+// AnotherNamespace
+namespace AnotherNamespace {
+// Function in AnotherNamespace
+void functionInAnotherNamespace() {}
+// Class in AnotherNamespace
+class ClassInAnotherNamespace {};
+}
+
+// JSON-INDEX: async function LoadIndex() {
+// JSON-INDEX-NEXT: return{
+// JSON-INDEX-NEXT: "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT: "Name": "",
+// JSON-INDEX-NEXT: "RefType": "default",
+// JSON-INDEX-NEXT: "Path": "",
+// JSON-INDEX-NEXT: "Children": [
+// JSON-INDEX-NEXT: {
+// JSON-INDEX-NEXT: "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT: "Name": "@nonymous_namespace",
+// JSON-INDEX-NEXT: "RefType": "namespace",
+// JSON-INDEX-NEXT: "Path": "@nonymous_namespace",
+// JSON-INDEX-NEXT: "Children": [
+// JSON-INDEX-NEXT: {
+// JSON-INDEX-NEXT: "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT: "Name": "AnonClass",
+// JSON-INDEX-NEXT: "RefType": "record",
+// JSON-INDEX-NEXT: "Path": "@nonymous_namespace",
+// JSON-INDEX-NEXT: "Children": []
+// JSON-INDEX-NEXT: }
+// JSON-INDEX-NEXT: ]
+// JSON-INDEX-NEXT: },
+// JSON-INDEX-NEXT: {
+// JSON-INDEX-NEXT: "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT: "Name": "AnotherNamespace",
+// JSON-INDEX-NEXT: "RefType": "namespace",
+// JSON-INDEX-NEXT: "Path": "AnotherNamespace",
+// JSON-INDEX-NEXT: "Children": [
+// JSON-INDEX-NEXT: {
+// JSON-INDEX-NEXT: "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT: "Name": "ClassInAnotherNamespace",
+// JSON-INDEX-NEXT: "RefType": "record",
+// JSON-INDEX-NEXT: "Path": "AnotherNamespace",
+// JSON-INDEX-NEXT: "Children": []
+// JSON-INDEX-NEXT: }
+// JSON-INDEX-NEXT: ]
+// JSON-INDEX-NEXT: },
+// JSON-INDEX-NEXT: {
+// JSON-INDEX-NEXT: "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT: "Name": "GlobalNamespace",
+// JSON-INDEX-NEXT: "RefType": "namespace",
+// JSON-INDEX-NEXT: "Path": "GlobalNamespace",
+// JSON-INDEX-NEXT: "Children": []
+// JSON-INDEX-NEXT: },
+// JSON-INDEX-NEXT: {
+// JSON-INDEX-NEXT: "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT: "Name": "PrimaryNamespace",
+//
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
ilovepi wrote: > I don't think this possible since the line directive matches the line of the > html or md file you pipe into FileCheck, which is different from the line > that's define in the source code. I've opted for just matching any number to > make the test case less brittle I’m not sure I follow. The LINE directive should work fine regardless of what output you’re checking. It should match the current line that the check is on, so if you do something like LINE-1 with the check right after the thing you’re trying to match, it should work fine. If you look at tests in Clang, LLVM , or compiler-rt you should find plenty of examples demonstrating the typical usage. https://github.com/llvm/llvm-project/pull/97681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add enum test (PR #97679)
@@ -0,0 +1,121 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --format=html --doxygen --output=%t --executor=standalone %s
+// RUN: clang-doc --format=md --doxygen --output=%t --executor=standalone %s
+// RUN: FileCheck %s < %t/GlobalNamespace/index.html -check-prefix=HTML-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/Animals.html
-check-prefix=HTML-ANIMAL
+// RUN: FileCheck %s < %t/Vehicles/index.html -check-prefix=HTML-VEHICLES
+// RUN: FileCheck %s < %t/GlobalNamespace/index.md -check-prefix=MD-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/Animals.md -check-prefix=MD-ANIMAL
+// RUN: FileCheck %s < %t/Vehicles/index.md -check-prefix=MD-VEHICLES
+
+
+/**
+ * @brief For specifying RGB colors
+ */
+enum Color {
+ Red, // Red
+ Green, // Green
+ Blue // Blue
+};
+
+/**
+ * @brief Shape Types
+ */
+enum Shapes {
+ // Circle
+ Circle,
+ // Rectangle
+ Rectangle,
+ // Triangle
+ Triangle
+};
+
+class Animals {
+public:
+ /**
+ * @brief specify what animal the class is
+ */
+ enum AnimalType {
+ Dog, // Man's best friend
+ Cat, // Man's other best friend
+ Iguana // A lizard
+ };
+};
+
+
+namespace Vehicles {
+/**
+ * @brief specify type of car
+ */
+enum Car {
+ Sedan, // Sedan
ilovepi wrote:
Shouldn’t these be documentation comments? So using /// or the doxygen style
comments?
My earlier point was that in LLVM and other projects, there are often
documentation comments attached the the enum entries. It would be good to test
that. If it doesn’t work, we can add a TODO and a tracking bug on GitHub.
https://github.com/llvm/llvm-project/pull/97679
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add enum test (PR #97679)
@@ -0,0 +1,121 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --format=html --doxygen --output=%t --executor=standalone %s
+// RUN: clang-doc --format=md --doxygen --output=%t --executor=standalone %s
+// RUN: FileCheck %s < %t/GlobalNamespace/index.html -check-prefix=HTML-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/Animals.html
-check-prefix=HTML-ANIMAL
+// RUN: FileCheck %s < %t/Vehicles/index.html -check-prefix=HTML-VEHICLES
+// RUN: FileCheck %s < %t/GlobalNamespace/index.md -check-prefix=MD-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/Animals.md -check-prefix=MD-ANIMAL
+// RUN: FileCheck %s < %t/Vehicles/index.md -check-prefix=MD-VEHICLES
+
+
+/**
+ * @brief For specifying RGB colors
+ */
+enum Color {
+ Red, // Red
+ Green, // Green
+ Blue // Blue
+};
+
+/**
+ * @brief Shape Types
+ */
+enum Shapes {
ilovepi wrote:
Maybe this can be ‘enum class’? We already have a normal enum above.
https://github.com/llvm/llvm-project/pull/97679
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add enum test (PR #97679)
@@ -0,0 +1,121 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --format=html --doxygen --output=%t --executor=standalone %s
+// RUN: clang-doc --format=md --doxygen --output=%t --executor=standalone %s
+// RUN: FileCheck %s < %t/GlobalNamespace/index.html -check-prefix=HTML-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/Animals.html
-check-prefix=HTML-ANIMAL
+// RUN: FileCheck %s < %t/Vehicles/index.html -check-prefix=HTML-VEHICLES
+// RUN: FileCheck %s < %t/GlobalNamespace/index.md -check-prefix=MD-INDEX
+// RUN: FileCheck %s < %t/GlobalNamespace/Animals.md -check-prefix=MD-ANIMAL
+// RUN: FileCheck %s < %t/Vehicles/index.md -check-prefix=MD-VEHICLES
+
+
+/**
+ * @brief For specifying RGB colors
+ */
+enum Color {
+ Red, // Red
+ Green, // Green
+ Blue // Blue
+};
+
+/**
+ * @brief Shape Types
+ */
+enum Shapes {
+ // Circle
+ Circle,
+ // Rectangle
+ Rectangle,
+ // Triangle
+ Triangle
+};
+
+class Animals {
+public:
+ /**
+ * @brief specify what animal the class is
+ */
+ enum AnimalType {
+ Dog, // Man's best friend
+ Cat, // Man's other best friend
+ Iguana // A lizard
+ };
+};
+
+
+namespace Vehicles {
+/**
+ * @brief specify type of car
+ */
+enum Car {
+ Sedan, // Sedan
+ SUV, // SUV
+ Pickup, // Pickup
+ Hatchback // Hatchback
+};
+}
+
+
+// HTML-INDEX: Global Namespace
+// HTML-INDEX: Enums
+// HTML-INDEX: enum Color
+// HTML-INDEX: Red
+// HTML-INDEX: Green
+// HTML-INDEX: Blue
+// HTML-INDEX: Defined at line {{.*}} of file
{{.*}}clang-tools-extra{{[\/]}}test{{[\/]}}clang-doc{{[\/]}}enum.cpp
ilovepi wrote:
```suggestion
// HTML-INDEX: Defined at line [[#]] of file
{{.*}}clang-tools-extra{{[\/]}}test{{[\/]}}clang-doc{{[\/]}}enum.cpp
```
i think this may be what you want to just match a number. It’s also possible to
use the LINE directive here, but that may be a bit harder to manage/keep in
sync. Probably, just matching the number here is enough, and another prefix can
check the line.
https://github.com/llvm/llvm-project/pull/97679
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -681,6 +683,7 @@ llvm::Error ClangDocBitcodeReader::readRecord(unsigned ID,
Reference *I) {
// Read a block of records into a single info.
template
llvm::Error ClangDocBitcodeReader::readBlock(unsigned ID, T I) {
+ llvm::TimeTraceScope("readBlock", "ClangDocBitcodeReader");
ilovepi wrote:
Have you looked at the profile with the new names? Is it easy to follow? I’d
expect it’s a bit harder now, since you’ve dropped some info.
>From what I can see, the typical usage of the name is something like “parse
>input files”. Mostly within our tools we seem to try to capture how long some
>task is taking, e.g. parsing files, constructing the ast, generating call
>graphs, writing out files, etc.
I’d consider structuring these more around the tasks your tracking, and perhaps
use the details to track more specific bits within a larger task or phase.
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -13,12 +13,17 @@
#include "clang/Index/USRGeneration.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/TimeProfiler.h"
namespace clang {
namespace doc {
void MapASTVisitor::HandleTranslationUnit(ASTContext ) {
+ if (CDCtx.FTimeTrace)
+llvm::timeTraceProfilerInitialize(CDCtx.Granularity, "clang-doc");
ilovepi wrote:
There’s lots of ways to initialize/teardown things so I’m fine with it the way
it is, but I wanted to know if there was a reason it wasn’t in those other
places.
I’d take another look at how this gets used in other tools, and follow whatever
seems to be common.
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add enum test (PR #97679)
@@ -0,0 +1,38 @@
+// RUN: clang-doc --format=html --doxygen --output=%t/docs
--executor=standalone %s
+// RUN: clang-doc --format=md --doxygen --output=%t/docs --executor=standalone
%s
+// RUN: FileCheck %s -input-file=%t/docs/GlobalNamespace/index.html
-check-prefix=HTML-INDEX
+// RUN: FileCheck %s -input-file=%t/docs/GlobalNamespace/index.md
-check-prefix=MD-INDEX
+
+/**
+ * @brief For specifying RGB colors
+ */
+enum Color {
+ Red, // Red enums
+ Green, // Green enums
+ Blue // Blue enums
ilovepi wrote:
These seem to be gone now, but isn’t it common to document the enum values? I
know we do that in several places in LLVM.
Thinking about other cases, it probably also worth having an “enum class”, an
enum within a class, and one in a namespace. I’d suggest also varying in at
least one case where the doc comments are: e.g., above the value vs on the same
line.
https://github.com/llvm/llvm-project/pull/97679
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
https://github.com/ilovepi edited https://github.com/llvm/llvm-project/pull/97681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
https://github.com/ilovepi edited https://github.com/llvm/llvm-project/pull/97681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
https://github.com/ilovepi edited https://github.com/llvm/llvm-project/pull/97681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
https://github.com/ilovepi commented: I think this mostly looks good, but I am concerned with the line number matching. It appears that whenever you’ve up’s the file you’ve had to update all the line numbers. Is it feasible to rewrite some of the checks to use the @LINE directive? Perhaps it also makes sense to move those checks close to the lines they expect to match? Another thought is to leave the test as is, but drop the line numbers(or just match any number) and have a small set of check in line with the code that checks the line numbers with the LINE directive and uses a new prefix. https://github.com/llvm/llvm-project/pull/97681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] modify basic-project test (PR #97684)
https://github.com/ilovepi approved this pull request. LGTM, but let’s update the pr title and description to match the current patch. Also, a title like modify tests isn’t very informative. Something like “Support markdown and simplify checks” seems more appropriate. Additionally, those two aspects seem like they could be separate PRs, though I won’t require that. https://github.com/llvm/llvm-project/pull/97684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Add another test project to clang-doc (PR #97518)
ilovepi wrote: Closing wasn’t necessary. The changes here could have just been made the the basic project instead of as new files. https://github.com/llvm/llvm-project/pull/97518 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [libc] Update libc namespace clang-tidy checks (PR #98424)
https://github.com/ilovepi closed https://github.com/llvm/llvm-project/pull/98424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [libc] Update libc namespace clang-tidy checks (PR #98424)
ilovepi wrote: Also, as an FYI to reviewers, I'm working on the FixIts for these as a separate patch. https://github.com/llvm/llvm-project/pull/98424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [libc] Update libc namespace clang-tidy checks (PR #98424)
@@ -30,20 +30,35 @@ void ImplementationInNamespaceCheck::check(
const auto *MatchedDecl =
Result.Nodes.getNodeAs("child_of_translation_unit");
const auto *NS = dyn_cast(MatchedDecl);
+
+ // LLVM libc declarations should be inside of a non-anonymous namespace.
if (NS == nullptr || NS->isAnonymousNamespace()) {
ilovepi wrote:
@PiotrZSL shouldn't his handle the anonymous namespace concern below?
https://github.com/llvm/llvm-project/pull/98424
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [libc] Update libc namespace clang-tidy checks (PR #98424)
@@ -30,20 +30,35 @@ void ImplementationInNamespaceCheck::check(
const auto *MatchedDecl =
Result.Nodes.getNodeAs("child_of_translation_unit");
const auto *NS = dyn_cast(MatchedDecl);
+
+ // LLVM libc declarations should be inside of a non-anonymous namespace.
if (NS == nullptr || NS->isAnonymousNamespace()) {
diag(MatchedDecl->getLocation(),
"declaration must be enclosed within the '%0' namespace")
-<< RequiredNamespaceMacroName;
+<< RequiredNamespaceDeclMacroName;
return;
}
+
+ // Enforce that the namespace is the result of macro expansion
if (Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) == false) {
diag(NS->getLocation(), "the outermost namespace should be the '%0' macro")
-<< RequiredNamespaceMacroName;
+<< RequiredNamespaceDeclMacroName;
return;
}
- if (NS->getName().starts_with(RequiredNamespaceStart) == false) {
+
+ // We want the macro to have [[gnu::visibility("hidden")]] as a prefix, but
+ // visibility is just an attribute in the AST construct, so we check that
+ // instead.
+ if (NS->getVisibility() != Visibility::HiddenVisibility) {
diag(NS->getLocation(), "the '%0' macro should start with '%1'")
-<< RequiredNamespaceMacroName << RequiredNamespaceStart;
+<< RequiredNamespaceDeclMacroName << RequiredNamespaceDeclStart;
+return;
+ }
+
+ // Lastly, make sure the namespace name actually has the __llvm_libc prefix
+ if (NS->getName().starts_with(RequiredNamespaceRefStart) == false) {
ilovepi wrote:
hmm, wait, don't we already check for that in line 35?
https://github.com/llvm/llvm-project/pull/98424
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [libc] Update libc namespace clang-tidy checks (PR #98424)
@@ -30,20 +30,35 @@ void ImplementationInNamespaceCheck::check(
const auto *MatchedDecl =
Result.Nodes.getNodeAs("child_of_translation_unit");
const auto *NS = dyn_cast(MatchedDecl);
+
+ // LLVM libc declarations should be inside of a non-anonymous namespace.
if (NS == nullptr || NS->isAnonymousNamespace()) {
diag(MatchedDecl->getLocation(),
"declaration must be enclosed within the '%0' namespace")
-<< RequiredNamespaceMacroName;
+<< RequiredNamespaceDeclMacroName;
return;
}
+
+ // Enforce that the namespace is the result of macro expansion
if (Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) == false) {
diag(NS->getLocation(), "the outermost namespace should be the '%0' macro")
-<< RequiredNamespaceMacroName;
+<< RequiredNamespaceDeclMacroName;
return;
}
- if (NS->getName().starts_with(RequiredNamespaceStart) == false) {
+
+ // We want the macro to have [[gnu::visibility("hidden")]] as a prefix, but
+ // visibility is just an attribute in the AST construct, so we check that
+ // instead.
+ if (NS->getVisibility() != Visibility::HiddenVisibility) {
diag(NS->getLocation(), "the '%0' macro should start with '%1'")
-<< RequiredNamespaceMacroName << RequiredNamespaceStart;
+<< RequiredNamespaceDeclMacroName << RequiredNamespaceDeclStart;
+return;
+ }
+
+ // Lastly, make sure the namespace name actually has the __llvm_libc prefix
+ if (NS->getName().starts_with(RequiredNamespaceRefStart) == false) {
ilovepi wrote:
Oh, good catch! Let me update this.
https://github.com/llvm/llvm-project/pull/98424
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
ilovepi wrote:
> Ok nevermind, disregard the above comment I was wrong about the mechanism of
> the bug. the source of this bug comes from the way clang-doc handles C code,
> particularly anonymous typedef in C. When clang-doc encounters an anonymous
> typedef in C it incorrectly serializes its a name. As a simple example
> running clang-doc on this file
>
> test.c
>
> ```
> /**
> * Foo anon typedef
> */
> typedef struct {} Foo;
> ```
>
> with cause it to interpret Foo as @nonymous_record_XXX.
>
> The reason why that is because when checking anonymous typedef name we're
> explicitly casting it to a CXXDecl. (see code
> [here](https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-doc/Serialize.cpp#L664))
> Since the above is c code it won't be cast to CXXRecordDecl and hences
> clang-doc will treat it as an anonymous record.
>
> This becomes a problem when we run the LLVM compilation database. As an
> example, a record that we were not properly serializing before was
> LLVMOrcCDependenceMapPair. When clang-doc runs the compilation database of
> LLVM it first comes across the file OrcV2CBindingsRemovableCode.c under the
> directory llvm/examples/OrcV2Examples/OrcV2CBindingsRemovableCode. This file
> causes recursiveASTVisitor to visit LLVMOrcCDependenceMapPair but because its
> c code its name is not found and it is incorrectly serialized into
> @nonymous_record_X. Normally this is not an issue for clang-doc, since
> we'll visit the record multiple times and the next time we visit a cpp file
> using LLVMOrcCDependenceMapPair , clang-doc will correctly fill in the name
> of the declaration. This also means that the order of the compilation also
> matters. If clang-doc had parsed a cpp file with LLVMOrcCDependenceMapPair
> than it will correctly serialized the record. This was why I had so much
> trouble reproducing the bug when I was using clang-doc --filter option to try
> and isolate the bug.
>
> Since this patch changes the behaviour to only visiting the a typedef
> declaration at most once, we now incorrectly serialized what was
> LLVMOrcCDependenceMapPair to @nonymous_record_X since on my machine it
> encounters LLVMOrcCDependenceMapPair first when its parsing
> OrcV2CBindingsRemovableCode.c.
>
> The workaround I added still works since when it comes to anonymous typedefs
> we skip out on memomization. It would be better if we just correctly handle
> the case of parsing anonymous typedef in C. But this approach is more
> conservative and safer since it reverts to old behaviour of clang-doc instead
> of introducing some other bug with parsing typedefs.
Thanks for the detailed analysis. With the current version of the patch, are
there any difference between what we get on main and what we get w/ this patch?
https://github.com/llvm/llvm-project/pull/96809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
@@ -53,30 +80,34 @@ template bool MapASTVisitor::mapDecl(const T
*D) {
}
bool MapASTVisitor::VisitNamespaceDecl(const NamespaceDecl *D) {
- return mapDecl(D);
+ return mapDecl(D, true);
}
-bool MapASTVisitor::VisitRecordDecl(const RecordDecl *D) { return mapDecl(D); }
+bool MapASTVisitor::VisitRecordDecl(const RecordDecl *D) {
+ return mapDecl(D, D->isThisDeclarationADefinition());
+}
-bool MapASTVisitor::VisitEnumDecl(const EnumDecl *D) { return mapDecl(D); }
+bool MapASTVisitor::VisitEnumDecl(const EnumDecl *D) {
+ return mapDecl(D, D->isThisDeclarationADefinition());
+}
bool MapASTVisitor::VisitCXXMethodDecl(const CXXMethodDecl *D) {
- return mapDecl(D);
+ return mapDecl(D, D->isThisDeclarationADefinition());
}
bool MapASTVisitor::VisitFunctionDecl(const FunctionDecl *D) {
// Don't visit CXXMethodDecls twice
if (isa(D))
return true;
- return mapDecl(D);
+ return mapDecl(D, D->isThisDeclarationADefinition());
}
bool MapASTVisitor::VisitTypedefDecl(const TypedefDecl *D) {
- return mapDecl(D);
+ return mapDecl(D, true);
ilovepi wrote:
same here and below.
https://github.com/llvm/llvm-project/pull/96809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
@@ -12,16 +12,31 @@
#include "clang/AST/Comment.h"
#include "clang/Index/USRGeneration.h"
#include "llvm/ADT/StringExtras.h"
-#include "llvm/Support/Error.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Mutex.h"
namespace clang {
namespace doc {
+static llvm::StringSet USRVisited;
+static llvm::sys::Mutex USRVisitedGuard;
+
+
+template bool isTypedefAnonRecord(const T* D) {
+ if (const auto *C = dyn_cast(D)) {
+if (const TypedefNameDecl *TD = C->getTypedefNameForAnonDecl()) {
+ return true;
+}
ilovepi wrote:
```suggestion
return C->getTypedefNameForAnonDecl();
```
Isn't this just the same. You still need the outer return, but this doesn't
need a branch and temporary.
https://github.com/llvm/llvm-project/pull/96809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
https://github.com/ilovepi commented: Seems like there's a few clang-format errors, so lets address those, too. https://github.com/llvm/llvm-project/pull/96809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
https://github.com/ilovepi edited https://github.com/llvm/llvm-project/pull/96809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
@@ -53,30 +80,34 @@ template bool MapASTVisitor::mapDecl(const T
*D) {
}
bool MapASTVisitor::VisitNamespaceDecl(const NamespaceDecl *D) {
- return mapDecl(D);
+ return mapDecl(D, true);
ilovepi wrote:
for constant params you should have a comment, per our style guide.
```suggestion
return mapDecl(D, /*isDefinition=*/true);
```
https://github.com/llvm/llvm-project/pull/96809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)
@@ -12,16 +12,21 @@
#include "clang/AST/Comment.h"
#include "clang/Index/USRGeneration.h"
#include "llvm/ADT/StringExtras.h"
-#include "llvm/Support/Error.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Mutex.h"
namespace clang {
namespace doc {
+static llvm::StringSet USRVisited;
ilovepi wrote:
```suggestion
static llvm::StringSet<> USRVisited;
```
There's a warning about type deduction (`-Wctad-maybe-unsupported`)
https://github.com/llvm/llvm-project/pull/96809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [libc] Update libc namespace clang-tidy checks (PR #98424)
@@ -3,60 +3,61 @@
#define MACRO_A "defining macros outside namespace is valid"
class ClassB;
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE' namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE_DECL' namespace
struct StructC {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE' namespace
-char *VarD = MACRO_A;
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE' namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE_DECL' namespace
+const char *VarD = MACRO_A;
ilovepi wrote:
I know this is a bit unrelated, but the tidy output was pretty polluted with
warnings due to the lack of `const` here. When I manually ran the tidy command
+ Filecheck, it seemed to fail due to those warnings, so I just fixed those
since I was changing these checks anyway.
https://github.com/llvm/llvm-project/pull/98424
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [libc] Update libc namespace clang-tidy checks (PR #98424)
https://github.com/ilovepi ready_for_review https://github.com/llvm/llvm-project/pull/98424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [libc] Update libc namespace clang-tidy checks (PR #98424)
https://github.com/ilovepi created
https://github.com/llvm/llvm-project/pull/98424
This patch updates the clang-tidy checks for llvm-libc to ensure that the
namespace macro used to declare the libc namespace is updated from
LIBC_NAMESPACE to LIBC_NAMESPACE_DECL which by default has hidden visibility.
Co-authored-by: Prabhu Rajesakeran
>From 2270c6ac1a775727bcb9766dd75646e20d120b0a Mon Sep 17 00:00:00 2001
From: prabhukr
Date: Mon, 8 Jul 2024 15:18:10 -0700
Subject: [PATCH] [libc] Update libc namespace clang-tidy checks
Namespace macro that should be used to declare a new namespace is
updated from LIBC_NAMESPACE to LIBC_NAMESPACE_DECL which by default has
hidden visibility (#97109). This commit updates the clang-tidy checks to
match the new policy.
---
.../llvmlibc/CalleeNamespaceCheck.cpp | 4 +-
.../ImplementationInNamespaceCheck.cpp| 23 +--
.../clang-tidy/llvmlibc/NamespaceConstants.h | 8 +++-
.../llvmlibc/implementation-in-namespace.rst | 20 -
.../llvmlibc/implementation-in-namespace.cpp | 41 ++-
5 files changed, 58 insertions(+), 38 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
index af977bff70f7c..caa19c595823f 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
@@ -51,7 +51,7 @@ void CalleeNamespaceCheck::check(const
MatchFinder::MatchResult ) {
// __llvm_libc, we're good.
const auto *NS = dyn_cast(getOutermostNamespace(FuncDecl));
if (NS && Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) &&
- NS->getName().starts_with(RequiredNamespaceStart))
+ NS->getName().starts_with(RequiredNamespaceRefStart))
return;
const DeclarationName = FuncDecl->getDeclName();
@@ -62,7 +62,7 @@ void CalleeNamespaceCheck::check(const
MatchFinder::MatchResult ) {
diag(UsageSiteExpr->getBeginLoc(),
"%0 must resolve to a function declared "
"within the namespace defined by the '%1' macro")
- << FuncDecl << RequiredNamespaceMacroName;
+ << FuncDecl << RequiredNamespaceRefMacroName;
diag(FuncDecl->getLocation(), "resolves to this declaration",
clang::DiagnosticIDs::Note);
diff --git
a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
index ae9819ed97502..bb436e4d12a30 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
@@ -30,20 +30,35 @@ void ImplementationInNamespaceCheck::check(
const auto *MatchedDecl =
Result.Nodes.getNodeAs("child_of_translation_unit");
const auto *NS = dyn_cast(MatchedDecl);
+
+ // LLVM libc declarations should be inside of a non-anonymous namespace.
if (NS == nullptr || NS->isAnonymousNamespace()) {
diag(MatchedDecl->getLocation(),
"declaration must be enclosed within the '%0' namespace")
-<< RequiredNamespaceMacroName;
+<< RequiredNamespaceDeclMacroName;
return;
}
+
+ // Enforce that the namespace is the result of macro expansion
if (Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) == false) {
diag(NS->getLocation(), "the outermost namespace should be the '%0' macro")
-<< RequiredNamespaceMacroName;
+<< RequiredNamespaceDeclMacroName;
return;
}
- if (NS->getName().starts_with(RequiredNamespaceStart) == false) {
+
+ // We want the macro to have [[gnu::visibility("hidden")]] as a prefix, but
+ // visibility is just an attribute in the AST construct, so we check that
+ // instead.
+ if (NS->getVisibility() != Visibility::HiddenVisibility) {
diag(NS->getLocation(), "the '%0' macro should start with '%1'")
-<< RequiredNamespaceMacroName << RequiredNamespaceStart;
+<< RequiredNamespaceDeclMacroName << RequiredNamespaceDeclStart;
+return;
+ }
+
+ // Lastly, make sure the namespace name actually has the __llvm_libc prefix
+ if (NS->getName().starts_with(RequiredNamespaceRefStart) == false) {
+diag(NS->getLocation(), "the '%0' macro expansion should start with '%1'")
+<< RequiredNamespaceDeclMacroName << RequiredNamespaceRefStart;
return;
}
}
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
b/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
index 7d4120085b866..83908a7875d03 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
+++ b/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
@@ -10,7 +10,11 @@
namespace clang::tidy::llvm_libc {
-const static llvm::StringRef RequiredNamespaceStart = "__llvm_libc";
-const static llvm::StringRef RequiredNamespaceMacroName = "LIBC_NAMESPACE";
+const static llvm::StringRef
[clang] [compiler-rt] [ubsan] Display correct runtime messages for negative _BitInt (PR #96240)
@@ -0,0 +1,42 @@
+// RUN: %clang -Wno-constant-conversion -Wno-array-bounds
-Wno-division-by-zero -Wno-shift-negative-value -Wno-shift-count-negative
-Wno-int-to-pointer-cast -O0
-fsanitize=alignment,array-bounds,bool,float-cast-overflow,implicit-integer-sign-change,implicit-signed-integer-truncation,implicit-unsigned-integer-truncation,integer-divide-by-zero,nonnull-attribute,null,nullability-arg,nullability-assign,nullability-return,pointer-overflow,returns-nonnull-attribute,shift-base,shift-exponent,signed-integer-overflow,unreachable,unsigned-integer-overflow,unsigned-shift-base,vla-bound
%s -o %t1 && %run %t1 2>&1 | FileCheck %s
+
+#include
+#include
+
+// In this test there is an expectation of assignment of _BitInt not producing
any output.
+uint32_t nullability_arg(_BitInt(37) *_Nonnull x)
+__attribute__((no_sanitize("address")))
+__attribute__((no_sanitize("memory"))) {
+ _BitInt(37) y = *(_BitInt(37) *)
+ return (y > 0) ? y : 0;
+}
+
+// In this test there is an expectation of ubsan not triggeting on returning
random address which is inside address space of the process.
+_BitInt(37) nonnull_attribute(__attribute__((nonnull)) _BitInt(37) * x)
+__attribute__((no_sanitize("address")))
+__attribute__((no_sanitize("memory"))) {
+ return *(_BitInt(37) *)
+}
+
+// In this test there is an expectation of assignment of uint32_t from
"invalid" _BitInt is not producing any output.
+uint32_t nullability_assign(_BitInt(7) * x)
+__attribute__((no_sanitize("address")))
+__attribute__((no_sanitize("memory"))) {
+ _BitInt(7) *_Nonnull y = x;
+ int32_t r = *(_BitInt(7) *)
+ return (r > 0) ? r : 0;
+}
+
+// In those examples the file is expected to compile with no diagnostics
ilovepi wrote:
yes.
https://github.com/llvm/llvm-project/pull/96240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] switched from using relative to absolute paths (PR #93281)
ilovepi wrote: any progress here? https://github.com/llvm/llvm-project/pull/93281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] modify basic-project test (PR #97684)
@@ -54,306 +54,249 @@
// JSON-INDEX-NEXT: };
// JSON-INDEX-NEXT: }
-// HTML-SHAPE:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT: class Shape
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT: class Shape
-// HTML-SHAPE-NEXT: Defined at line 8 of file {{.*}}Shape.h
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT: Provides a common interface for different
types of shapes.
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT: Functions
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT: ~Shape
-// HTML-SHAPE-NEXT: public void ~Shape()
-// HTML-SHAPE-NEXT: Defined at line 13 of file {{.*}}Shape.h
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT: area
-// HTML-SHAPE-NEXT: public double area()
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT: perimeter
-// HTML-SHAPE-NEXT: public double perimeter()
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT: Functions
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT: ~Shape
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT: area
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT: perimeter
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
-// HTML-SHAPE-NEXT:
+// HTML-SHAPE: class Shape
+// HTML-SHAPE: Defined at line 8 of file {{.*}}Shape.h
+// HTML-SHAPE:
+// HTML-SHAPE:
ilovepi wrote:
Do you need to match these? are they important? same w/ all the lines that only
have tags...
https://github.com/llvm/llvm-project/pull/97684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] modify basic-project test (PR #97684)
https://github.com/ilovepi edited https://github.com/llvm/llvm-project/pull/97684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] modify basic-project test (PR #97684)
https://github.com/ilovepi commented: This is headed in the right direction, but I'm not sure we want to match so many of the single line tags ... they don't seem particularly valuable from a testing standpoint. If you think we're missing tests for the HTML structure, then we should have some tests that focus on just that aspect that can be a bit simpler in nature than a full project. Ultimately, I'd like to see a subset of tests that veryify that we do the documentation correctly, and a set of tests that exercise the format enough to be sure we're emiting well structured markdown/yaml/html. https://github.com/llvm/llvm-project/pull/97684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
@@ -0,0 +1,340 @@ +// RUN: clang-doc --format=html --output=%t/docs --executor=standalone %s +// RUN: clang-doc --format=md --output=%t/docs --executor=standalone %s ilovepi wrote: Why no `mkdir`? I presume because clang-doc will make it, but the test shouldn't rely on that behavior. Plus we want to be sure each run is isolated, so starting w/ `rm -rf %t && mkdir %t` is a good practice if you need a directory. also, probably don't need `docs`... https://github.com/llvm/llvm-project/pull/97681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add nested namespace test case (PR #97681)
@@ -0,0 +1,340 @@ +// RUN: clang-doc --format=html --output=%t/docs --executor=standalone %s +// RUN: clang-doc --format=md --output=%t/docs --executor=standalone %s +// RUN: FileCheck %s -input-file=%t/docs/index_json.js -check-prefix=JSON-INDEX +// RUN: FileCheck %s -input-file=%t/docs/@nonymous_namespace/AnonClass.html -check-prefix=HTML-ANON-CLASS +// RUN: FileCheck %s -input-file=%t/docs/@nonymous_namespace/index.html -check-prefix=HTML-ANON-INDEX +// RUN: FileCheck %s -input-file=%t/docs/AnotherNamespace/ClassInAnotherNamespace.html -check-prefix=HTML-ANOTHER-CLASS +// RUN: FileCheck %s -input-file=%t/docs/AnotherNamespace/index.html -check-prefix=HTML-ANOTHER-INDEX +// RUN: FileCheck %s -input-file=%t/docs/GlobalNamespace/index.html -check-prefix=HTML-GLOBAL-INDEX +// RUN: FileCheck %s -input-file=%t/docs/PrimaryNamespace/NestedNamespace/ClassInNestedNamespace.html -check-prefix=HTML-NESTED-CLASS +// RUN: FileCheck %s -input-file=%t/docs/PrimaryNamespace/NestedNamespace/index.html -check-prefix=HTML-NESTED-INDEX +// RUN: FileCheck %s -input-file=%t/docs/PrimaryNamespace/index.html -check-prefix=HTML-PRIMARY-INDEX +// RUN: FileCheck %s -input-file=%t/docs/PrimaryNamespace/ClassInPrimaryNamespace.html -check-prefix=HTML-PRIMARY-CLASS +// RUN: FileCheck %s -input-file=%t/docs/@nonymous_namespace/AnonClass.md -check-prefix=MD-ANON-CLASS +// RUN: FileCheck %s -input-file=%t/docs/@nonymous_namespace/index.md -check-prefix=MD-ANON-INDEX +// RUN: FileCheck %s -input-file=%t/docs/AnotherNamespace/ClassInAnotherNamespace.md -check-prefix=MD-ANOTHER-CLASS +// RUN: FileCheck %s -input-file=%t/docs/AnotherNamespace/index.md -check-prefix=MD-ANOTHER-INDEX +// RUN: FileCheck %s -input-file=%t/docs/GlobalNamespace/index.md -check-prefix=MD-GLOBAL-INDEX +// RUN: FileCheck %s -input-file=%t/docs/PrimaryNamespace/NestedNamespace/ClassInNestedNamespace.md -check-prefix=MD-NESTED-CLASS +// RUN: FileCheck %s -input-file=%t/docs/PrimaryNamespace/NestedNamespace/index.md -check-prefix=MD-NESTED-INDEX +// RUN: FileCheck %s -input-file=%t/docs/PrimaryNamespace/index.md -check-prefix=MD-PRIMARY-INDEX +// RUN: FileCheck %s -input-file=%t/docs/PrimaryNamespace/ClassInPrimaryNamespace.md -check-prefix=MD-PRIMARY-CLASS +// RUN: FileCheck %s -input-file=%t/docs/all_files.md -check-prefix=MD-ALL-FILES +// RUN: FileCheck %s -input-file=%t/docs/index.md -check-prefix=MD-INDEX ilovepi wrote: nit: -input-file https://github.com/llvm/llvm-project/pull/97681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add enum test (PR #97679)
@@ -0,0 +1,38 @@ +// RUN: clang-doc --format=html --doxygen --output=%t/docs --executor=standalone %s +// RUN: clang-doc --format=md --doxygen --output=%t/docs --executor=standalone %s +// RUN: FileCheck %s -input-file=%t/docs/GlobalNamespace/index.html -check-prefix=HTML-INDEX +// RUN: FileCheck %s -input-file=%t/docs/GlobalNamespace/index.md -check-prefix=MD-INDEX ilovepi wrote: Do these not work w/o `-input-file`? https://github.com/llvm/llvm-project/pull/97679 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add enum test (PR #97679)
@@ -0,0 +1,38 @@
+// RUN: clang-doc --format=html --doxygen --output=%t/docs
--executor=standalone %s
+// RUN: clang-doc --format=md --doxygen --output=%t/docs --executor=standalone
%s
+// RUN: FileCheck %s -input-file=%t/docs/GlobalNamespace/index.html
-check-prefix=HTML-INDEX
+// RUN: FileCheck %s -input-file=%t/docs/GlobalNamespace/index.md
-check-prefix=MD-INDEX
+
+/**
+ * @brief For specifying RGB colors
+ */
+enum Color {
+ Red, // Red enums
+ Green, // Green enums
+ Blue // Blue enums
ilovepi wrote:
should these be doc comments?
https://github.com/llvm/llvm-project/pull/97679
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add enum test (PR #97679)
@@ -0,0 +1,38 @@ +// RUN: clang-doc --format=html --doxygen --output=%t/docs --executor=standalone %s +// RUN: clang-doc --format=md --doxygen --output=%t/docs --executor=standalone %s ilovepi wrote: This can't work right, can it? you haven't made `%t`, you also don't need `docs`, right? It would be good if I didn't have to point these out in every patch... I've given this piece of feedback in almost all of your patches. https://github.com/llvm/llvm-project/pull/97679 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -670,6 +671,7 @@ llvm::Error ClangDocBitcodeReader::readRecord(unsigned ID,
T I) {
template <>
llvm::Error ClangDocBitcodeReader::readRecord(unsigned ID, Reference *I) {
+ llvm::TimeTraceScope("clang-doc", "readRecord Reference");
ilovepi wrote:
Why `readRecord Reference`? If you want to describe the fact that you're in
`readRecord`, just do that, otherwise, if its a logical thing, `Read Record`.
If `reference` is important, its fine to leave, but its not clear to me why.
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -362,6 +395,17 @@ Example usage for a project using a compile commands
database:
if (Err) {
llvm::outs() << "warning: " << toString(std::move(Err)) << "\n";
}
-
+ llvm::timeTraceProfilerEnd();
+
+ if (FTimeTrace) {
+std::error_code EC;
+llvm::raw_fd_ostream OS("clang-doc-tracing.json", EC,
+llvm::sys::fs::OF_Text);
+if (!EC) {
+ llvm::timeTraceProfilerWrite(OS);
+} else {
+ llvm::errs() << "Error opening file: " << EC.message() << "\n";
ilovepi wrote:
shouldn't we be returning an error value from main here?
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -99,6 +100,16 @@ URL of repository that hosts code.
Used for links to definition locations.)"),
llvm::cl::cat(ClangDocCategory));
+static llvm::cl::opt FTimeTrace("ftime-trace", llvm::cl::desc(R"(
+Turn on time profiler. Generates clang-doc-tracing.json)"),
+ llvm::cl::init(false),
+ llvm::cl::cat(ClangDocCategory));
+
+static llvm::cl::opt FTimeGranularity("ftime-gran", llvm::cl::desc(R"(
ilovepi wrote:
```suggestion
static llvm::cl::opt FTimeGranularity("ftime-granularity",
llvm::cl::desc(R"(
```
I haven't seen this used elsewhere, is it common in other components? I think
its more typical to pick/set the granularity for a tool as a constant. Even if
we keep it, it should be a hidden option.
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -30,6 +35,7 @@ template bool MapASTVisitor::mapDecl(const T *D)
{
if (D->getParentFunctionOrMethod())
return true;
+ llvm::timeTraceProfilerBegin("emit info phase", "emit info");
ilovepi wrote:
nit: try to be consistent w/ the logging style and capitalization of your
messages. It seems to be all over the place.
More concretely, though, what does `emit info` mean/imply here?
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -316,32 +342,39 @@ Example usage for a project using a compile commands
database:
std::move(ReadInfos->begin(), ReadInfos->end(),
std::back_inserter(Infos));
}
+ llvm::timeTraceProfilerEnd();
+ llvm::timeTraceProfilerBegin("merging bitcode phase", "merging");
auto Reduced = doc::mergeInfos(Infos);
if (!Reduced) {
llvm::errs() << llvm::toString(Reduced.takeError());
return;
}
+ llvm::timeTraceProfilerEnd();
// Add a reference to this Info in the Index
{
std::lock_guard Guard(IndexMutex);
clang::doc::Generator::addInfoToIndex(CDCtx.Idx, Reduced.get().get());
}
-
ilovepi wrote:
nit: unrelated change.
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -681,6 +683,7 @@ llvm::Error ClangDocBitcodeReader::readRecord(unsigned ID,
Reference *I) {
// Read a block of records into a single info.
template
llvm::Error ClangDocBitcodeReader::readBlock(unsigned ID, T I) {
+ llvm::TimeTraceScope("readBlock", "ClangDocBitcodeReader");
ilovepi wrote:
Actually, looking at these, I'm not sure I follow the organizational structure
of these. How did you decide what's the `Name` and what's the `Detail`? is
there some logic to this?
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -480,12 +480,15 @@ mergeInfos(std::vector> );
struct ClangDocContext {
ClangDocContext() = default;
ClangDocContext(tooling::ExecutionContext *ECtx, StringRef ProjectName,
- bool PublicOnly, StringRef OutDirectory, StringRef
SourceRoot,
+ bool PublicOnly, bool FTimeTrace, int Granularity,
ilovepi wrote:
These new params should probably go at the end of the arguments list. Likely
they should have a default value, too.
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -30,7 +30,7 @@ ClangDocContext
getClangDocContext(std::vector UserStylesheets = {},
StringRef RepositoryUrl = "") {
ClangDocContext CDCtx{
- {}, "test-project", {}, {}, {}, RepositoryUrl, UserStylesheets};
+ {}, "test-project", {}, {}, {}, {}, {}, RepositoryUrl, UserStylesheets};
ilovepi wrote:
If the new parameters go at the end, and have default values, then you don't
need to updated this.
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -13,12 +13,17 @@
#include "clang/Index/USRGeneration.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/TimeProfiler.h"
namespace clang {
namespace doc {
void MapASTVisitor::HandleTranslationUnit(ASTContext ) {
+ if (CDCtx.FTimeTrace)
+llvm::timeTraceProfilerInitialize(CDCtx.Granularity, "clang-doc");
ilovepi wrote:
Why here and not say in the lambda in clang-doc main, or in the constructor for
the visitor?
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
@@ -362,6 +395,17 @@ Example usage for a project using a compile commands
database:
if (Err) {
llvm::outs() << "warning: " << toString(std::move(Err)) << "\n";
}
-
+ llvm::timeTraceProfilerEnd();
+
+ if (FTimeTrace) {
+std::error_code EC;
+llvm::raw_fd_ostream OS("clang-doc-tracing.json", EC,
+llvm::sys::fs::OF_Text);
+if (!EC) {
+ llvm::timeTraceProfilerWrite(OS);
+} else {
ilovepi wrote:
nit: no braces for single statement conditions.
https://github.com/llvm/llvm-project/pull/97644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
https://github.com/ilovepi requested changes to this pull request. https://github.com/llvm/llvm-project/pull/97644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add ftime profiling (PR #97644)
https://github.com/ilovepi edited https://github.com/llvm/llvm-project/pull/97644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] Add another test project to clang-doc (PR #97518)
https://github.com/ilovepi requested changes to this pull request. I'm a bit confused by the "advanced project", as it largely seems to be minor extensions to the `basic project`. I would expect that an "advanced project" would use a complicated structure and be much larger. IMO if you're only adding some more language features, then they should be added to the existing `basic_project` already in tree. https://github.com/llvm/llvm-project/pull/97518 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [llvm][misexpect] Update MisExpect to use provenance tracking metadata (PR #86610)
@@ -151,15 +151,9 @@ void verifyMisExpect(Instruction , ArrayRef RealWeights, uint64_t TotalBranchWeight = LikelyBranchWeight + (UnlikelyBranchWeight * NumUnlikelyTargets); - // FIXME: When we've addressed sample profiling, restore the assertion - // - // We cannot calculate branch probability if either of these invariants aren't - // met. However, MisExpect diagnostics should not prevent code from compiling, - // so we simply forgo emitting diagnostics here, and return early. - // assert((TotalBranchWeight >= LikelyBranchWeight) && (TotalBranchWeight > 0) - // && "TotalBranchWeight is less than the Likely branch weight"); - if ((TotalBranchWeight == 0) || (TotalBranchWeight <= LikelyBranchWeight)) -return; + // Failing this assert means that we have corrupted metadata. + assert((TotalBranchWeight >= LikelyBranchWeight) && (TotalBranchWeight > 0) && ilovepi wrote: it was introduced after we found an issue w/ thinLTO, since sample profiling can run multiple times. We then introduced an early return to avoid division by zero, but now it should be safe to just use the assert, since we should only ever process branches w/ the provenance tag. for now, I guess let's leave it as an assert, since I don't expect it to fire, and users don't have control of it. https://github.com/llvm/llvm-project/pull/86610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [llvm][misexpect] Update MisExpect to use provenance tracking metadata (PR #86610)
@@ -151,15 +151,9 @@ void verifyMisExpect(Instruction , ArrayRef RealWeights, uint64_t TotalBranchWeight = LikelyBranchWeight + (UnlikelyBranchWeight * NumUnlikelyTargets); - // FIXME: When we've addressed sample profiling, restore the assertion - // - // We cannot calculate branch probability if either of these invariants aren't - // met. However, MisExpect diagnostics should not prevent code from compiling, - // so we simply forgo emitting diagnostics here, and return early. - // assert((TotalBranchWeight >= LikelyBranchWeight) && (TotalBranchWeight > 0) - // && "TotalBranchWeight is less than the Likely branch weight"); - if ((TotalBranchWeight == 0) || (TotalBranchWeight <= LikelyBranchWeight)) -return; + // Failing this assert means that we have corrupted metadata. + assert((TotalBranchWeight >= LikelyBranchWeight) && (TotalBranchWeight > 0) && ilovepi wrote: IIRC many of the places where we check metadata for corruption or format incompatibility just use asserts, but I'm fine if its a diagnostic. Let me know if you have a preference, as I'm fine either way. That said, if we're going to make this a diagnostic, we should probably move it out of MisExpect and put it on some other code path where we're examining branch weights, since MisExpect is one of the least used compilation modes. WDYT? https://github.com/llvm/llvm-project/pull/86610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Handle class member access expressions with valid nested-name-specifiers that become invalid after lookup (PR #98167)
ilovepi wrote: Thanks for running this down! https://github.com/llvm/llvm-project/pull/98167 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] fix paths by hard coding path to share (PR #98099)
https://github.com/ilovepi approved this pull request. LGTM, modulo my comments on the commit message and title, and premerge checks passing. https://github.com/llvm/llvm-project/pull/98099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] fix paths by hard coding path to share (PR #98099)
ilovepi wrote: I'd rephrase your commit message and title in the following way. ``` [clang-doc][cmake] Fix asset directory location for other generator types This patch fixes how clang-doc copies asset files for the build to match the install location for all CMake generator types. The previous version of this patch did not account for standalone builds of clang that may use older LLVM installs, ane which do not have the `LLVM_SHARE_OUTPUT_INTDIR` defined. Instead, we create an equivalent path using `LLVM_RUNTIME_OUTPUT_INTDIR`. Fixes #97507 ``` You may want to adjust the line length to match 80 columns, since I just eyeballed it. https://github.com/llvm/llvm-project/pull/98099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc][nfc] Avoid constructing SmallString in ToString method (PR #96921)
https://github.com/ilovepi closed https://github.com/llvm/llvm-project/pull/96921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [llvm][misexpect] Update MisExpect to use provenance tracking metadata (PR #86610)
ilovepi wrote: ping https://github.com/llvm/llvm-project/pull/86610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CMake][Fuchsia] -fvisibility=hidden for baremetal runtimes (PR #98049)
@@ -321,7 +321,7 @@ foreach(target
armv6m-unknown-eabi;armv7m-unknown-eabi;armv8m-unknown-eabi)
set(RUNTIMES_${target}_CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
set(RUNTIMES_${target}_CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY CACHE
STRING "")
foreach(lang C;CXX;ASM)
-set(RUNTIMES_${target}_CMAKE_${lang}_FLAGS "--target=${target} -mthumb
-Wno-atomic-alignment" CACHE STRING "")
+set(RUNTIMES_${target}_CMAKE_${lang}_FLAGS "--target=${target} -mthumb
-Wno-atomic-alignment -fvisibility=hidden" CACHE STRING "")
ilovepi wrote:
Does this need the TODO as well?
https://github.com/llvm/llvm-project/pull/98049
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [clang-doc] fix bug introduced by asset test (PR #97540)
https://github.com/ilovepi approved this pull request. https://github.com/llvm/llvm-project/pull/97540 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] fix bug introduced by asset test (PR #97540)
ilovepi wrote: > > I believe (and the pre-merge testing seems to validate my belief) that this > > breaks the case where you build with ninja + Visual Studio. Also, I believe > > the Xcode generator is similar to the Visual Studio generator in that they > > both are multi-config generators, so that configuration is likely failing > > as well. > > @ilovepi > > I don't have access to a mac could you run build for checking clang-doc in > xcode to see if it reports any errors? while I have access to a mac, I don't have the ability to use XCode due to company policy. https://github.com/llvm/llvm-project/pull/97540 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] fix bug introduced by asset test (PR #97540)
@@ -52,4 +56,4 @@ add_custom_target(copy-clang-doc-assets COMMENT "Copying Clang-Doc Assets" ) set_target_properties(copy-clang-doc-assets PROPERTIES FOLDER "Clang-Doc/Assets") -add_dependencies(clang-doc copy-clang-doc-assets) +add_dependencies(clang-doc copy-clang-doc-assets) ilovepi wrote: nit: I think your editor may be modifying the end of the file here unnecessarily. https://github.com/llvm/llvm-project/pull/97540 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] fix bug introduced by asset test (PR #97540)
@@ -25,7 +25,11 @@ set(assets
)
set(asset_dir "${CMAKE_CURRENT_SOURCE_DIR}/../assets")
-set(resource_dir "${CMAKE_BINARY_DIR}/share/clang-doc")
+if(MSVC)
ilovepi wrote:
Doesn't this reflect the compiler and not the build system/project structure?
I'm pretty sure you can use Ninja and build w/ MSVC, right? In that case, I'm
not sure this is the right approach.
https://github.com/llvm/llvm-project/pull/97540
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] fix bug introduced by asset test (PR #97540)
ilovepi wrote: seems to fail on windows CI. https://github.com/llvm/llvm-project/pull/97540 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-doc] add more test to clang-doc (PR #97518)
@@ -0,0 +1,31 @@
+#include "Array.h"
+
+// Implementation of Array
+
+/**
+* Initializes all elements of the array to their default value.
+*/
+template
+Array::Array() {
+ // Implementation stub
+}
+
+/**
+* Array access operator for Array
+* Provides read and write access to elements in the array.
+* This implementation does not perform bounds checking
+*/
+template
+T& Array::operator[](int index) {
+ // Implementation stub
+ static T dummy;
+ return dummy;
ilovepi wrote:
In some of these cases, it may be a useful test to include documentation
strings on internal statement, to verify that we're not including them in the
generated documentation, or if we're supporting those kinds of doc comments,
that we're testing that they work correctly.
https://github.com/llvm/llvm-project/pull/97518
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
