Skip to content

Commit

Permalink
Updated ApiView to use clang-15 to work around MSVC STL changes; fixe…
Browse files Browse the repository at this point in the history
…d warning in keyvault-admin ApiView (#5901)

* Updated ApiView to use clang-15 to work around MSVC STL changes; fixed warning in keyvault-admin ApiView

* Move to latest clang 15; added error diagnostic for using namespace in public header

* Update tools/apiview/parsers/cpp-api-parser/ApiViewProcessor/AstNode.cpp

Co-authored-by: Rick Winter <[email protected]>

* Added a note about the vcpkg port

---------

Co-authored-by: Rick Winter <[email protected]>
  • Loading branch information
LarryOsterman and RickWinter authored Apr 5, 2023
1 parent 9d27383 commit 71e0603
Show file tree
Hide file tree
Showing 25 changed files with 1,403 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ void AzureClassesDatabase::CreateApiViewMessage(
}
case ApiViewMessages::InternalTypesInNonCorePackage: {
newMessage.DiagnosticId = "CPA0007";
newMessage.DiagnosticText
= "'internal' types declared in a non-common package. Consider putting the type in the '_detail' namespace.";
newMessage.DiagnosticText = "'internal' types declared in a non-common package. Consider "
"putting the type in the '_detail' namespace.";
newMessage.Level = ApiViewMessage::MessageLevel::Warning;
break;
}
Expand All @@ -67,6 +67,14 @@ void AzureClassesDatabase::CreateApiViewMessage(
newMessage.Level = ApiViewMessage::MessageLevel::Info;
break;
}
case ApiViewMessages::UsingDirectiveFound: {
newMessage.DiagnosticId = "CPA0009";
newMessage.DiagnosticText = "Using Namespace directive found in header file. ";
newMessage.HelpLinkUri
= "https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rs-using-directive";
newMessage.Level = ApiViewMessage::MessageLevel::Error;
break;
}
}
newMessage.TargetId = targetId;
m_diagnostics.push_back(std::move(newMessage));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,5 @@ enum class ApiViewMessages
ProtectedFieldsInFinalClass, // Protected fields in final class
InternalTypesInNonCorePackage, // Internal types in a non-core package
ImplicitConstructor, // Constructor for a type is not marked "explicit".
UsingDirectiveFound, // "using namespace" directive found.
};
Original file line number Diff line number Diff line change
Expand Up @@ -2148,6 +2148,39 @@ class AstFriend : public AstNode {
}
};

class AstUsingDirective : public AstNode {
std::string m_namedNamespace;

public:
AstUsingDirective(
UsingDirectiveDecl const* usingDirective,
AzureClassesDatabase* const azureClassesDatabase,
std::shared_ptr<TypeHierarchy::TypeHierarchyNode> parentNode)
: AstNode(usingDirective), m_namedNamespace{usingDirective->getNominatedNamespaceAsWritten()
->getQualifiedNameAsString()}
{
azureClassesDatabase->CreateApiViewMessage(
ApiViewMessages::UsingDirectiveFound, m_namedNamespace);
}
void DumpNode(AstDumper* dumper, DumpNodeOptions const& dumpOptions) override
{
if (dumpOptions.NeedsLeftAlign)
{
dumper->LeftAlign();
}
dumper->InsertKeyword("using");
dumper->InsertWhitespace();
dumper->InsertKeyword("namespace");
dumper->InsertWhitespace();
dumper->InsertTypeName(m_namedNamespace, m_namedNamespace);
dumper->InsertPunctuation(';');
if (dumpOptions.NeedsTrailingNewline)
{
dumper->Newline();
}
}
};

class AstEnumerator : public AstNamedNode {
std::unique_ptr<AstExpr> m_initializer;

Expand Down Expand Up @@ -2748,6 +2781,12 @@ std::unique_ptr<AstNode> AstNode::Create(
case Decl::Kind::Friend:
return std::make_unique<AstFriend>(cast<FriendDecl>(decl), azureClassesDatabase, parentNode);

case Decl::Kind::UsingDirective:
// A "UsingDirective" is a "using namespace" directive. We consider this an error
// condition, add an AstNode so the error appears in the ApiView.
return std::make_unique<AstUsingDirective>(
cast<UsingDirectiveDecl>(decl), azureClassesDatabase, parentNode);

case Decl::Kind::NamespaceAlias:
return nullptr;
// return std::make_unique<AstNamespaceAlias>(cast<NamespaceAliasDecl>(decl,
Expand All @@ -2759,7 +2798,6 @@ std::unique_ptr<AstNode> AstNode::Create(
// azureClassesDatabase));
case Decl::Kind::Using:
return nullptr;
// return std::make_unique<AstUsing>(cast<UsingDecl>(decl));
default: {
llvm::errs() << raw_ostream::Colors::RED << "Unknown DECL node "
<< cast<NamedDecl>(decl)->getNameAsString()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ target_include_directories(ApiViewProcessor PRIVATE ${LLVM_INCLUDE_DIRS})
llvm_map_components_to_libnames(llvm_libs
Support
Option
WindowsDriver
FrontEndOpenMP
)

Expand All @@ -41,6 +42,7 @@ clangLex
clangParse
clangDriver
clangSema
clangSupport
clangFrontend
clangSerialization
clangTooling
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ bool ApiViewProcessorImpl::CollectCppClassesVisitor::ShouldCollectNamedDecl(
shouldCollect = true;
}
}

// We don't even want to consider any types which are a member of a class.
if (shouldCollect)
{
Expand Down Expand Up @@ -399,7 +398,7 @@ int ApiViewProcessorImpl::ProcessApiView()
// compilation database. Use the CurrentDirectorySetter to preserve and restore the current
// directory across calls into the clang tooling.
CurrentDirectorySetter currentDirectory{std::filesystem::current_path()};

// clang really likes all input paths to be absolute paths, so use the fiilesystem to
// canonicalize the input filename and source location.
std::filesystem::path tempFile = std::filesystem::temp_directory_path();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ add_executable(parseTests
TestCases/TemplateTests.cpp
TestCases/ClassesWithInternalAndDetail.cpp
TestCases/ExpressionTests.cpp
)
TestCases/UsingNamespace.cpp )
add_dependencies(parseTests ApiViewProcessor)

target_include_directories(parseTests PRIVATE ${ApiViewProcessor_SOURCE_DIR})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: MIT

#include <chrono>
#include <cstddef>
#include <map>
#include <string>
#include <vector>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// SPDX-License-Identifier: MIT

#include <chrono>
#include <map>
#include <string>
#include <vector>

namespace Test { namespace Inner {
class Fred {};
}} // namespace Test::Inner

using namespace Test::Inner;

namespace A { namespace AB { namespace ABCD {
char* GlobalFunctionInAABABCD(int character);
}}} // namespace A::AB::ABCD
44 changes: 39 additions & 5 deletions tools/apiview/parsers/cpp-api-parser/ParseTests/tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ TEST_F(TestParser, CompileWithErrors)
}
}


struct NsDumper : AstDumper
{
// Inherited via AstDumper
Expand Down Expand Up @@ -451,11 +450,11 @@ TEST_F(TestParser, Class1)

auto& db = processor.GetClassesDatabase();
EXPECT_EQ(16ul, db->GetAstNodeMap().size());

NsDumper dumper;
db->DumpClassDatabase(&dumper);
EXPECT_EQ(31ul, dumper.Messages.size());

size_t internalTypes = 0;
for (const auto& msg : dumper.Messages)
{
Expand All @@ -465,7 +464,7 @@ TEST_F(TestParser, Class1)
}
}
EXPECT_EQ(internalTypes, 8ul);

EXPECT_TRUE(SyntaxCheckClassDb(db, "Classes1.cpp"));
}
TEST_F(TestParser, Class2)
Expand Down Expand Up @@ -529,7 +528,42 @@ TEST_F(TestParser, Templates)

auto& db = processor.GetClassesDatabase();
// Until we get parsing types working correctly, we can't do the syntax check tests.
// EXPECT_TRUE(SyntaxCheckClassDb(db, "Template1.cpp"));
// EXPECT_TRUE(SyntaxCheckClassDb(db, "Template1.cpp"));
}

TEST_F(TestParser, UsingNamespace)
{
ApiViewProcessor processor("tests", R"({
"sourceFilesToProcess": [
"UsingNamespace.cpp"
],
"additionalIncludeDirectories": [],
"additionalCompilerSwitches": null,
"allowInternal": false,
"includeDetail": false,
"includePrivate": false,
"filterNamespace": null
}
)"_json);

EXPECT_EQ(processor.ProcessApiView(), 0);

auto& db = processor.GetClassesDatabase();
EXPECT_TRUE(SyntaxCheckClassDb(db, "UsingNamespace1.cpp"));

NsDumper dumper;
db->DumpClassDatabase(&dumper);
EXPECT_EQ(1ul, dumper.Messages.size());

size_t usingNamespaces = 0;
for (const auto& msg : dumper.Messages)
{
if (msg.DiagnosticId == "CPA0009")
{
usingNamespaces += 1;
}
}
EXPECT_EQ(usingNamespaces, 1ul);
}

#if 0
Expand Down
6 changes: 6 additions & 0 deletions tools/apiview/parsers/cpp-api-parser/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,9 @@ displayed in the ApiView tool.
This tokenized representation is modeled in the following example JSON document found [here](https://github.com/Azure/azure-sdk-tools/blob/main/src/dotnet/APIView/apiview_token_gist.json)
When this JSON file is parsed by the API View tool, it will create the following text in the ApiView:
![API View snippet defining `ClassLibrary1.dll`](https://i.imgur.com/ikfRmLM.png)

#### VCPKG notes

The `ParseAzureSdkCpp` tool uses a custom port for clang-15 because vcpkg does not currently have a port for clang-15 in the public repository.

The clang-15 port files were taken from the [vcpkg repository](https://github.com/microsoft/vcpkg/pull/26902).
Loading

0 comments on commit 71e0603

Please sign in to comment.