| Index: tools/clang/plugins/FindBadConstructsConsumer.cpp
|
| diff --git a/tools/clang/plugins/FindBadConstructsConsumer.cpp b/tools/clang/plugins/FindBadConstructsConsumer.cpp
|
| index 4dc305e4a4e3d61bd751ce65f44812348e8335bc..83a18b0206f7d1ac6371a5fead21a7ba05f91aa9 100644
|
| --- a/tools/clang/plugins/FindBadConstructsConsumer.cpp
|
| +++ b/tools/clang/plugins/FindBadConstructsConsumer.cpp
|
| @@ -78,13 +78,14 @@ bool IsGtestTestFixture(const CXXRecordDecl* decl) {
|
| // have to be true, but it probably will be.
|
| // TODO(dcheng): Add a warning to force virtual to always appear first ;-)
|
| FixItHint FixItRemovalForVirtual(const SourceManager& manager,
|
| + const LangOptions& lang_opts,
|
| const CXXMethodDecl* method) {
|
| SourceRange range(method->getLocStart());
|
| // Get the spelling loc just in case it was expanded from a macro.
|
| SourceRange spelling_range(manager.getSpellingLoc(range.getBegin()));
|
| // Sanity check that the text looks like virtual.
|
| StringRef text = clang::Lexer::getSourceText(
|
| - CharSourceRange::getTokenRange(spelling_range), manager, LangOptions());
|
| + CharSourceRange::getTokenRange(spelling_range), manager, lang_opts);
|
| if (text.trim() != "virtual")
|
| return FixItHint();
|
| return FixItHint::CreateRemoval(range);
|
| @@ -290,12 +291,30 @@ void FindBadConstructsConsumer::CheckCtorDtorWeight(
|
| if (it->hasInlineBody()) {
|
| if (it->isCopyConstructor() &&
|
| !record->hasUserDeclaredCopyConstructor()) {
|
| - emitWarning(record_location,
|
| - "Complex class/struct needs an explicit out-of-line "
|
| - "copy constructor.");
|
| + // In general, implicit constructors are generated on demand. But
|
| + // in the Windows component build, dllexport causes instantiation of
|
| + // the copy constructor which means that this fires on many more
|
| + // classes. For now, suppress this on dllexported classes.
|
| + // (This does mean that windows component builds will not emit this
|
| + // warning in some cases where it is emitted in other configs, but
|
| + // that's the better tradeoff at this point).
|
| + // TODO(dcheng): With the RecursiveASTVisitor, these warnings might
|
| + // be emitted on other platforms too, reevaluate if we want to keep
|
| + // surpressing this then http://crbug.com/467288
|
| + if (!record->hasAttr<DLLExportAttr>())
|
| + emitWarning(record_location,
|
| + "Complex class/struct needs an explicit out-of-line "
|
| + "copy constructor.");
|
| } else {
|
| - emitWarning(it->getInnerLocStart(),
|
| - "Complex constructor has an inlined body.");
|
| + // See the comment in the previous branch about copy constructors.
|
| + // This does the same for implicit move constructors.
|
| + bool is_likely_compiler_generated_dllexport_move_ctor =
|
| + it->isMoveConstructor() &&
|
| + !record->hasUserDeclaredMoveConstructor() &&
|
| + record->hasAttr<DLLExportAttr>();
|
| + if (!is_likely_compiler_generated_dllexport_move_ctor)
|
| + emitWarning(it->getInnerLocStart(),
|
| + "Complex constructor has an inlined body.");
|
| }
|
| } else if (it->isInlined() && !it->isInlineSpecified() &&
|
| !it->isDeleted() && (!it->isCopyOrMoveConstructor() ||
|
| @@ -354,6 +373,15 @@ bool FindBadConstructsConsumer::IsMethodInBannedOrTestingNamespace(
|
| return false;
|
| }
|
|
|
| +SuppressibleDiagnosticBuilder
|
| +FindBadConstructsConsumer::ReportIfSpellingLocNotIgnored(
|
| + SourceLocation loc,
|
| + unsigned diagnostic_id) {
|
| + return SuppressibleDiagnosticBuilder(
|
| + &diagnostic(), loc, diagnostic_id,
|
| + InBannedDirectory(instance().getSourceManager().getSpellingLoc(loc)));
|
| +}
|
| +
|
| // Checks that virtual methods are correctly annotated, and have no body in a
|
| // header file.
|
| void FindBadConstructsConsumer::CheckVirtualMethods(
|
| @@ -411,6 +439,7 @@ void FindBadConstructsConsumer::CheckVirtualSpecifiers(
|
| return;
|
|
|
| SourceManager& manager = instance().getSourceManager();
|
| + const LangOptions& lang_opts = instance().getLangOpts();
|
|
|
| // Complain if a method is annotated virtual && (override || final).
|
| if (has_virtual && (override_attr || final_attr)) {
|
| @@ -418,13 +447,11 @@ void FindBadConstructsConsumer::CheckVirtualSpecifiers(
|
| // Note this is just an educated guess: the assumption here is that any
|
| // macro for declaring methods will probably be at the start of the method's
|
| // source range.
|
| - if (!InBannedDirectory(manager.getSpellingLoc(method->getLocStart()))) {
|
| - diagnostic().Report(method->getLocStart(),
|
| - diag_redundant_virtual_specifier_)
|
| - << "'virtual'"
|
| - << (override_attr ? static_cast<Attr*>(override_attr) : final_attr)
|
| - << FixItRemovalForVirtual(manager, method);
|
| - }
|
| + ReportIfSpellingLocNotIgnored(method->getLocStart(),
|
| + diag_redundant_virtual_specifier_)
|
| + << "'virtual'"
|
| + << (override_attr ? static_cast<Attr*>(override_attr) : final_attr)
|
| + << FixItRemovalForVirtual(manager, lang_opts, method);
|
| }
|
|
|
| // Complain if a method is an override and is not annotated with override or
|
| @@ -435,11 +462,8 @@ void FindBadConstructsConsumer::CheckVirtualSpecifiers(
|
| if (method->hasInlineBody()) {
|
| loc = method->getBody()->getSourceRange().getBegin();
|
| } else {
|
| - // TODO(dcheng): We should probably use ASTContext's LangOptions here.
|
| - LangOptions lang_options;
|
| - loc = Lexer::getLocForEndOfToken(
|
| - manager.getSpellingLoc(range.getEnd()), 0,
|
| - manager, lang_options);
|
| + loc = Lexer::getLocForEndOfToken(manager.getSpellingLoc(range.getEnd()),
|
| + 0, manager, lang_opts);
|
| // The original code used the ending source loc of TypeSourceInfo's
|
| // TypeLoc. Unfortunately, this breaks down in the presence of attributes.
|
| // Attributes often appear at the end of a TypeLoc, e.g.
|
| @@ -455,11 +479,11 @@ void FindBadConstructsConsumer::CheckVirtualSpecifiers(
|
| for (SourceLocation l = loc.getLocWithOffset(-1);
|
| l != manager.getLocForStartOfFile(manager.getFileID(loc));
|
| l = l.getLocWithOffset(-1)) {
|
| - l = Lexer::GetBeginningOfToken(l, manager, lang_options);
|
| + l = Lexer::GetBeginningOfToken(l, manager, lang_opts);
|
| Token token;
|
| // getRawToken() returns *true* on failure. In that case, just give up
|
| // and don't bother generating a possibly incorrect fix-it.
|
| - if (Lexer::getRawToken(l, token, manager, lang_options, true)) {
|
| + if (Lexer::getRawToken(l, token, manager, lang_opts, true)) {
|
| loc = SourceLocation();
|
| break;
|
| }
|
| @@ -471,25 +495,28 @@ void FindBadConstructsConsumer::CheckVirtualSpecifiers(
|
| }
|
| }
|
| }
|
| + // Again, only emit the warning if it doesn't originate from a macro in
|
| + // a system header.
|
| if (loc.isValid()) {
|
| - diagnostic().Report(loc, diag_method_requires_override_)
|
| + ReportIfSpellingLocNotIgnored(loc, diag_method_requires_override_)
|
| << FixItHint::CreateInsertion(loc, " override");
|
| } else {
|
| - diagnostic().Report(range.getBegin(), diag_method_requires_override_);
|
| + ReportIfSpellingLocNotIgnored(range.getBegin(),
|
| + diag_method_requires_override_);
|
| }
|
| }
|
|
|
| if (final_attr && override_attr) {
|
| - diagnostic().Report(override_attr->getLocation(),
|
| - diag_redundant_virtual_specifier_)
|
| + ReportIfSpellingLocNotIgnored(override_attr->getLocation(),
|
| + diag_redundant_virtual_specifier_)
|
| << override_attr << final_attr
|
| << FixItHint::CreateRemoval(override_attr->getRange());
|
| }
|
|
|
| if (final_attr && !is_override) {
|
| - diagnostic().Report(method->getLocStart(),
|
| - diag_base_method_virtual_and_final_)
|
| - << FixItRemovalForVirtual(manager, method)
|
| + ReportIfSpellingLocNotIgnored(method->getLocStart(),
|
| + diag_base_method_virtual_and_final_)
|
| + << FixItRemovalForVirtual(manager, lang_opts, method)
|
| << FixItHint::CreateRemoval(final_attr->getRange());
|
| }
|
| }
|
| @@ -501,9 +528,27 @@ void FindBadConstructsConsumer::CheckVirtualBodies(
|
| if (method->hasBody() && method->hasInlineBody()) {
|
| if (CompoundStmt* cs = dyn_cast<CompoundStmt>(method->getBody())) {
|
| if (cs->size()) {
|
| - emitWarning(cs->getLBracLoc(),
|
| - "virtual methods with non-empty bodies shouldn't be "
|
| - "declared inline.");
|
| + SourceLocation loc = cs->getLBracLoc();
|
| + // CR_BEGIN_MSG_MAP_EX and BEGIN_SAFE_MSG_MAP_EX try to be compatible
|
| + // to BEGIN_MSG_MAP(_EX). So even though they are in chrome code,
|
| + // we can't easily fix them, so explicitly whitelist them here.
|
| + bool emit = true;
|
| + if (loc.isMacroID()) {
|
| + SourceManager& manager = instance().getSourceManager();
|
| + if (InBannedDirectory(manager.getSpellingLoc(loc)))
|
| + emit = false;
|
| + else {
|
| + StringRef name = Lexer::getImmediateMacroName(
|
| + loc, manager, instance().getLangOpts());
|
| + if (name == "CR_BEGIN_MSG_MAP_EX" ||
|
| + name == "BEGIN_SAFE_MSG_MAP_EX")
|
| + emit = false;
|
| + }
|
| + }
|
| + if (emit)
|
| + emitWarning(loc,
|
| + "virtual methods with non-empty bodies shouldn't be "
|
| + "declared inline.");
|
| }
|
| }
|
| }
|
| @@ -529,7 +574,7 @@ void FindBadConstructsConsumer::CountType(const Type* type,
|
| bool whitelisted_template = false;
|
|
|
| // HACK: I'm at a loss about how to get the syntax checker to get
|
| - // whether a template is exterened or not. For the first pass here,
|
| + // whether a template is externed or not. For the first pass here,
|
| // just do retarded string comparisons.
|
| if (TemplateDecl* decl = name.getAsTemplateDecl()) {
|
| std::string base_name = decl->getNameAsString();
|
|
|