From 8dc86ea991d6c647faa10043afc88f520443575f Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 4 Sep 2019 15:18:26 -0400 Subject: [PATCH 1/3] main: Amend inclusion order --- atdna/main.cpp | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/atdna/main.cpp b/atdna/main.cpp index 3f3ad92..2a8af6e 100644 --- a/atdna/main.cpp +++ b/atdna/main.cpp @@ -1,19 +1,22 @@ +#include #include -#include -#include "clang/AST/ASTConsumer.h" -#include "clang/AST/RecursiveASTVisitor.h" -#include "clang/Frontend/CompilerInstance.h" -#include "clang/Frontend/FrontendAction.h" -#include "clang/Frontend/Utils.h" -#include "clang/Tooling/Tooling.h" -#include "clang/Lex/Preprocessor.h" -#include "clang/Sema/Sema.h" -#include "clang/AST/RecordLayout.h" -#include "clang/AST/DeclCXX.h" -#include "clang/AST/TypeLoc.h" -#include "clang/Basic/Version.h" -#include "llvm/Support/Format.h" -#include "llvm/Support/CommandLine.h" +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include using namespace std::literals; From 607f99fa1afba646d706c43fa60cc674658a2d01 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 4 Sep 2019 15:23:44 -0400 Subject: [PATCH 2/3] main: Amend variable shadowing warnings --- atdna/main.cpp | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/atdna/main.cpp b/atdna/main.cpp index 2a8af6e..9c12106 100644 --- a/atdna/main.cpp +++ b/atdna/main.cpp @@ -137,15 +137,15 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { const auto* vType = static_cast(field->getType().getTypePtr()); if (vType->isVectorType()) { const auto* eType = static_cast(vType->getElementType().getTypePtr()); - const uint64_t width = context.getTypeInfo(eType).Width; - if (!eType->isBuiltinType() || !eType->isFloatingPoint() || (width != 32 && width != 64)) + const uint64_t typeWidth = context.getTypeInfo(eType).Width; + if (!eType->isBuiltinType() || !eType->isFloatingPoint() || (typeWidth != 32 && typeWidth != 64)) continue; if (vType->getNumElements() == 2) { - return width / 8 * 2; + return typeWidth / 8 * 2; } else if (vType->getNumElements() == 3) { - return width / 8 * 3; + return typeWidth / 8 * 3; } else if (vType->getNumElements() == 4) { - return width / 8 * 4; + return typeWidth / 8 * 4; } } } @@ -687,18 +687,18 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { } else { const clang::NamedDecl* nd = tsDecl->getTemplatedDecl(); if (const clang::CXXRecordDecl* rd = clang::dyn_cast_or_null(nd)) { - std::string baseDNA; - if (isDNARecord(rd, baseDNA)) + std::string baseDNA2; + if (isDNARecord(rd, baseDNA2)) { outputNodes.emplace_back(NodeType::Do, fieldName, GetOpString(fieldName, propIdExpr), false); + } } } - } - - else if (regType->getTypeClass() == clang::Type::Record) { + } else if (regType->getTypeClass() == clang::Type::Record) { const clang::CXXRecordDecl* cxxRDecl = regType->getAsCXXRecordDecl(); - std::string baseDNA; - if (cxxRDecl && isDNARecord(cxxRDecl, baseDNA)) + std::string baseDNA2; + if (cxxRDecl && isDNARecord(cxxRDecl, baseDNA2)) { outputNodes.emplace_back(NodeType::Do, fieldName, GetOpString(fieldName, propIdExpr), false); + } } } @@ -995,20 +995,18 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { } else { const clang::NamedDecl* nd = tsDecl->getTemplatedDecl(); if (const clang::CXXRecordDecl* rd = clang::dyn_cast_or_null(nd)) { - std::string baseDNA; - if (isDNARecord(rd, baseDNA)) { + std::string baseDNA2; + if (isDNARecord(rd, baseDNA2)) { fileOut << " AT_PROP_CASE(" << propIdExpr << "):\n" << " Do" << GetOpString(fieldName, propIdExpr) << ";\n" << " return true;\n"; } } } - } - - else if (regType->getTypeClass() == clang::Type::Record) { + } else if (regType->getTypeClass() == clang::Type::Record) { const clang::CXXRecordDecl* cxxRDecl = regType->getAsCXXRecordDecl(); - std::string baseDNA; - if (cxxRDecl && isDNARecord(cxxRDecl, baseDNA)) { + std::string baseDNA2; + if (cxxRDecl && isDNARecord(cxxRDecl, baseDNA2)) { fileOut << " AT_PROP_CASE(" << propIdExpr << "):\n" << " Do" << GetOpString(fieldName, propIdExpr) << ";\n" << " return true;\n"; From 0dcf0cec03ad6bdd1e137175ed9ad02d1d1da5ce Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 4 Sep 2019 15:28:00 -0400 Subject: [PATCH 3/3] main: Avoid unnecessary string churn Reduces the amount of overall allocation churn due to string concatenation by avoiding redundant string temporaries. --- atdna/main.cpp | 169 +++++++++++++++++++++++++++++-------------------- 1 file changed, 99 insertions(+), 70 deletions(-) diff --git a/atdna/main.cpp b/atdna/main.cpp index 9c12106..e1cb6ec 100644 --- a/atdna/main.cpp +++ b/atdna/main.cpp @@ -167,7 +167,7 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { static std::string GetPropIdExpr(const clang::FieldDecl* field, const std::string& fieldName) { std::string fieldStr = GetFieldString(fieldName); - std::string propIdExpr = "\"" + fieldStr + "\""; + std::string propIdExpr = "\""s.append(fieldStr).append(1, '\"'); for (clang::Attr* attr : field->attrs()) { if (clang::AnnotateAttr* annot = clang::dyn_cast_or_null(attr)) { llvm::StringRef textRef = annot->getAnnotation(); @@ -187,22 +187,39 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { static std::string GetOpString(const std::string& fieldName, const std::string& propIdExpr, const std::string& endianExpr) { - return "(athena::io::PropId(" + propIdExpr + "), " + fieldName + ", s)"; + return "(athena::io::PropId(") + .append(propIdExpr) + .append("), ") + .append(fieldName) + .append(", s)"); } static std::string GetOpString(const std::string& fieldName, const std::string& propIdExpr) { - return "(athena::io::PropId(" + propIdExpr + "), " + fieldName + ", s)"; + return "(athena::io::PropId("s.append(propIdExpr).append("), ").append(fieldName).append(", s)"); } static std::string GetVectorOpString(const std::string& fieldName, const std::string& propIdExpr, const std::string& sizeExpr, const std::string& endianExpr) { - return "(athena::io::PropId(" + propIdExpr + "), " + fieldName + ", " + sizeExpr + ", s)"; + return "(athena::io::PropId(") + .append(propIdExpr) + .append("), ") + .append(fieldName) + .append(", ") + .append(sizeExpr) + .append(", s)"); } static std::string GetVectorOpString(const std::string& fieldName, const std::string& propIdExpr, const std::string& sizeExpr) { - return "(athena::io::PropId(" + propIdExpr + "), " + fieldName + ", " + sizeExpr + ", s)"; + return "(athena::io::PropId("s.append(propIdExpr) + .append("), ") + .append(fieldName) + .append(", ") + .append(sizeExpr) + .append(", s)"); } static void RecurseNestedTypeName(const clang::DeclContext* decl, std::string& templateStmt, std::string& qualType) { @@ -223,7 +240,7 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { templateStmt += ", "; qualType += ", "; } - templateStmt += "class "s + tpParm->getName().data(); + templateStmt += "class "s.append(tpParm->getName().str()); qualType += tpParm->getName(); needsComma = true; } else if (const clang::NonTypeTemplateParmDecl* nonTypeParm = @@ -232,7 +249,7 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { templateStmt += ", "; qualType += ", "; } - templateStmt += nonTypeParm->getType().getAsString() + ' ' + nonTypeParm->getName().data(); + templateStmt += nonTypeParm->getType().getAsString().append(1, ' ').append(nonTypeParm->getName().str()); qualType += nonTypeParm->getName(); needsComma = true; } @@ -280,10 +297,13 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { int numTuples = int(specParms.size()) / numParms; for (const auto& parent : parentSpecializations) { for (int i = 0; i < numTuples; ++i) { - if (parent.first.empty()) - specializations.emplace_back(std::string(rec->getName().data()) + '<', 1); - else - specializations.emplace_back(parent.first + "::" + rec->getName().data() + '<', parent.second + 1); + if (parent.first.empty()) { + specializations.emplace_back(std::string(rec->getName().str()).append(1, '<'), 1); + } else { + auto specialization = + std::string(parent.first).append("::").append(rec->getName().str()).append(1, '<'); + specializations.emplace_back(std::move(specialization), parent.second + 1); + } bool needsComma = false; for (auto it = specParms.begin() + i * numParms; it != specParms.end() && it != specParms.begin() + (i + 1) * numParms; ++it) { @@ -293,7 +313,7 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { specializations.back().first += trimmed; needsComma = true; } - specializations.back().first += ">"; + specializations.back().first += '>'; } } foundSpecializations = true; @@ -304,16 +324,20 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { } } - if (!foundSpecializations) + if (!foundSpecializations) { for (const auto& parent : parentSpecializations) { if (const clang::NamedDecl* namedDecl = clang::dyn_cast_or_null(decl)) { - if (parent.first.empty()) - specializations.emplace_back(namedDecl->getName().data(), parent.second); - else - specializations.emplace_back(parent.first + "::" + namedDecl->getName().data(), parent.second); - } else + if (parent.first.empty()) { + specializations.emplace_back(namedDecl->getName().str(), parent.second); + } else { + specializations.emplace_back(std::string(parent.first).append("::").append(namedDecl->getName().str()), + parent.second); + } + } else { specializations.push_back(parent); + } } + } } static std::vector> GetNestedTypeSpecializations(const clang::DeclContext* decl) { @@ -339,8 +363,8 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { std::string m_fieldName; std::string m_ioOp; bool m_squelched = false; - OutputNode(NodeType type, const std::string& fieldName, const std::string& ioOp, bool squelched) - : m_type(type), m_fieldName(fieldName), m_ioOp(ioOp), m_squelched(squelched) {} + OutputNode(NodeType type, std::string fieldName, std::string ioOp, bool squelched) + : m_type(type), m_fieldName(std::move(fieldName)), m_ioOp(std::move(ioOp)), m_squelched(squelched) {} }; std::vector outputNodes; @@ -392,24 +416,24 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { } std::string ioOp; - bool isDNAType = false; for (const clang::TemplateArgument& arg : *tsType) { if (arg.getKind() == clang::TemplateArgument::Type) { - if (defaultEndian) + if (defaultEndian) { ioOp = GetOpString(fieldName, propIdExpr); - else + } else { ioOp = GetOpString(fieldName, propIdExpr, endianExprStr); + } } } if (ioOp.empty()) { clang::DiagnosticBuilder diag = context.getDiagnostics().Report(field->getLocation(), AthenaError); - diag.AddString("Unable to use type '" + tsDecl->getName().str() + "' with Athena"); + diag.AddString("Unable to use type '"s.append(tsDecl->getName().str()).append("' with Athena")); diag.AddSourceRange(clang::CharSourceRange(field->getSourceRange(), true)); continue; } - outputNodes.emplace_back(NodeType::Do, fieldName, ioOp, false); + outputNodes.emplace_back(NodeType::Do, std::move(fieldName), std::move(ioOp), false); } else if (!tsDecl->getName().compare("Vector")) { llvm::APSInt endian(64, -1); std::string endianExprStr; @@ -460,20 +484,20 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { clang::QualType templateType; std::string ioOp; - bool isDNAType = false; for (const clang::TemplateArgument& arg : *tsType) { if (arg.getKind() == clang::TemplateArgument::Type) { templateType = arg.getAsType().getCanonicalType(); - if (defaultEndian) + if (defaultEndian) { ioOp = GetVectorOpString(fieldName, propIdExpr, sizeExpr); - else + } else { ioOp = GetVectorOpString(fieldName, propIdExpr, sizeExpr, endianExprStr); + } } } if (ioOp.empty()) { clang::DiagnosticBuilder diag = context.getDiagnostics().Report(field->getLocation(), AthenaError); - diag.AddString("Unable to use type '" + templateType.getAsString() + "' with Athena"); + diag.AddString("Unable to use type '"s.append(templateType.getAsString()).append("' with Athena")); diag.AddSourceRange(clang::CharSourceRange(field->getSourceRange(), true)); continue; } @@ -485,7 +509,7 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { continue; } - outputNodes.emplace_back(NodeType::Do, fieldName, ioOp, false); + outputNodes.emplace_back(NodeType::Do, std::move(fieldName), std::move(ioOp), false); } else if (!tsDecl->getName().compare("Buffer")) { const clang::Expr* sizeExpr = nullptr; std::string sizeExprStr; @@ -517,8 +541,7 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { } std::string ioOp = GetVectorOpString(fieldName, propIdExpr, sizeExprStr); - - outputNodes.emplace_back(NodeType::Do, fieldName, ioOp, false); + outputNodes.emplace_back(NodeType::Do, std::move(fieldName), std::move(ioOp), false); } else if (!tsDecl->getName().compare("String")) { std::string sizeExprStr; for (const clang::TemplateArgument& arg : *tsType) { @@ -540,12 +563,13 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { } std::string ioOp; - if (!sizeExprStr.empty()) + if (!sizeExprStr.empty()) { ioOp = GetVectorOpString(fieldName, propIdExpr, sizeExprStr); - else + } else { ioOp = GetOpString(fieldName, propIdExpr); + } - outputNodes.emplace_back(NodeType::Do, fieldName, ioOp, false); + outputNodes.emplace_back(NodeType::Do, std::move(fieldName), std::move(ioOp), false); } else if (!tsDecl->getName().compare("WString")) { llvm::APSInt endian(64, -1); std::string endianExprStr; @@ -588,18 +612,20 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { std::string ioOp; if (!sizeExprStr.empty()) { - if (defaultEndian) + if (defaultEndian) { ioOp = GetVectorOpString(fieldName, propIdExpr, sizeExprStr); - else + } else { ioOp = GetVectorOpString(fieldName, propIdExpr, sizeExprStr, endianExprStr); + } } else { - if (defaultEndian) + if (defaultEndian) { ioOp = GetOpString(fieldName, propIdExpr); - else + } else { ioOp = GetOpString(fieldName, propIdExpr, endianExprStr); + } } - outputNodes.emplace_back(NodeType::Do, fieldName, ioOp, false); + outputNodes.emplace_back(NodeType::Do, std::move(fieldName), std::move(ioOp), false); } else if (!tsDecl->getName().compare("Seek")) { size_t idx = 0; std::string offsetExprStr; @@ -653,15 +679,16 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { continue; } - if (directionVal == 0) - outputNodes.emplace_back(NodeType::DoSeek, fieldName, "("s + offsetExprStr + ", athena::Begin, s)", - false); - else if (directionVal == 1) - outputNodes.emplace_back(NodeType::DoSeek, fieldName, "("s + offsetExprStr + ", athena::Current, s)", - false); - else if (directionVal == 2) - outputNodes.emplace_back(NodeType::DoSeek, fieldName, "("s + offsetExprStr + ", athena::End, s)", - false); + if (directionVal == 0) { + outputNodes.emplace_back(NodeType::DoSeek, std::move(fieldName), + "("s.append(offsetExprStr).append(", athena::Begin, s)"), false); + } else if (directionVal == 1) { + outputNodes.emplace_back(NodeType::DoSeek, std::move(fieldName), + "("s.append(offsetExprStr).append(", athena::Current, s)"), false); + } else if (directionVal == 2) { + outputNodes.emplace_back(NodeType::DoSeek, std::move(fieldName), + "("s.append(offsetExprStr).append(", athena::End, s)"), false); + } } else if (!tsDecl->getName().compare("Align")) { llvm::APSInt align(64, 0); bool bad = false; @@ -680,16 +707,17 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { if (bad) continue; - int64_t alignVal = align.getSExtValue(); + const int64_t alignVal = align.getSExtValue(); if (alignVal) { - outputNodes.emplace_back(NodeType::DoAlign, fieldName, "("s + align.toString(10, true) + ", s)", false); + outputNodes.emplace_back(NodeType::DoAlign, std::move(fieldName), + "("s.append(align.toString(10, true)).append(", s)"), false); } } else { const clang::NamedDecl* nd = tsDecl->getTemplatedDecl(); if (const clang::CXXRecordDecl* rd = clang::dyn_cast_or_null(nd)) { std::string baseDNA2; if (isDNARecord(rd, baseDNA2)) { - outputNodes.emplace_back(NodeType::Do, fieldName, GetOpString(fieldName, propIdExpr), false); + outputNodes.emplace_back(NodeType::Do, std::move(fieldName), GetOpString(fieldName, propIdExpr), false); } } } @@ -697,7 +725,7 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { const clang::CXXRecordDecl* cxxRDecl = regType->getAsCXXRecordDecl(); std::string baseDNA2; if (cxxRDecl && isDNARecord(cxxRDecl, baseDNA2)) { - outputNodes.emplace_back(NodeType::Do, fieldName, GetOpString(fieldName, propIdExpr), false); + outputNodes.emplace_back(NodeType::Do, std::move(fieldName), GetOpString(fieldName, propIdExpr), false); } } } @@ -784,7 +812,6 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { } std::string ioOp; - bool isDNAType = false; for (const clang::TemplateArgument& arg : *tsType) { if (arg.getKind() == clang::TemplateArgument::Type) { if (defaultEndian) @@ -796,7 +823,7 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { if (ioOp.empty()) { clang::DiagnosticBuilder diag = context.getDiagnostics().Report(field->getLocation(), AthenaError); - diag.AddString("Unable to use type '" + tsDecl->getName().str() + "' with Athena"); + diag.AddString("Unable to use type '"s.append(tsDecl->getName().str()).append("' with Athena")); diag.AddSourceRange(clang::CharSourceRange(field->getSourceRange(), true)); continue; } @@ -844,7 +871,6 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { clang::QualType templateType; std::string ioOp; - bool isDNAType = false; for (const clang::TemplateArgument& arg : *tsType) { if (arg.getKind() == clang::TemplateArgument::Type) { templateType = arg.getAsType().getCanonicalType(); @@ -857,7 +883,7 @@ class ATDNAEmitVisitor : public clang::RecursiveASTVisitor { if (ioOp.empty()) { clang::DiagnosticBuilder diag = context.getDiagnostics().Report(field->getLocation(), AthenaError); - diag.AddString("Unable to use type '" + templateType.getAsString() + "' with Athena"); + diag.AddString("Unable to use type '"s.append(templateType.getAsString()).append("' with Athena")); diag.AddSourceRange(clang::CharSourceRange(field->getSourceRange(), true)); continue; } @@ -1169,24 +1195,27 @@ int main(int argc, const char** argv) { if (Help) llvm::cl::PrintHelpMessage(); - std::vector args = {"clang-tool", + std::vector args = { + "clang-tool", #ifdef __linux__ - "--gcc-toolchain=/usr", + "--gcc-toolchain=/usr", #endif - "-fsyntax-only", - "-std=c++1z", - "-D__atdna__=1", - "-Wno-expansion-to-defined", - "-Wno-nullability-completeness", - "-Werror=shadow-field", - "-I" XSTR(INSTALL_PREFIX) "/lib/clang/" CLANG_VERSION_STRING "/include", - "-I" XSTR(INSTALL_PREFIX) "/include/Athena"}; - for (int a = 1; a < argc; ++a) - args.push_back(argv[a]); + "-fsyntax-only", + "-std=c++1z", + "-D__atdna__=1", + "-Wno-expansion-to-defined", + "-Wno-nullability-completeness", + "-Werror=shadow-field", + "-I" XSTR(INSTALL_PREFIX) "/lib/clang/" CLANG_VERSION_STRING "/include", + "-I" XSTR(INSTALL_PREFIX) "/include/Athena", + }; + for (int a = 1; a < argc; ++a) { + args.emplace_back(argv[a]); + } llvm::IntrusiveRefCntPtr fman(new clang::FileManager(clang::FileSystemOptions())); ATDNAAction* action = new ATDNAAction(); - clang::tooling::ToolInvocation TI(args, action, fman.get()); + clang::tooling::ToolInvocation TI(std::move(args), action, fman.get()); if (!TI.run()) return 1;