Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(48)

Side by Side Diff: tools/clang/plugins/FindBadConstructsConsumer.cpp

Issue 1108173002: Roll //build, //native_client, and a few more targets of opportunity. Base URL: git@github.com:domokit/mojo.git@master
Patch Set: Test fix Created 5 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "FindBadConstructsConsumer.h" 5 #include "FindBadConstructsConsumer.h"
6 6
7 #include "clang/Frontend/CompilerInstance.h" 7 #include "clang/Frontend/CompilerInstance.h"
8 #include "clang/AST/Attr.h" 8 #include "clang/AST/Attr.h"
9 #include "clang/Lex/Lexer.h" 9 #include "clang/Lex/Lexer.h"
10 #include "llvm/Support/raw_ostream.h" 10 #include "llvm/Support/raw_ostream.h"
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after
63 return UnwrapType(elaborated->getNamedType().getTypePtr()); 63 return UnwrapType(elaborated->getNamedType().getTypePtr());
64 if (const TypedefType* typedefed = dyn_cast<TypedefType>(type)) 64 if (const TypedefType* typedefed = dyn_cast<TypedefType>(type))
65 return UnwrapType(typedefed->desugar().getTypePtr()); 65 return UnwrapType(typedefed->desugar().getTypePtr());
66 return type; 66 return type;
67 } 67 }
68 68
69 bool IsGtestTestFixture(const CXXRecordDecl* decl) { 69 bool IsGtestTestFixture(const CXXRecordDecl* decl) {
70 return decl->getQualifiedNameAsString() == "testing::Test"; 70 return decl->getQualifiedNameAsString() == "testing::Test";
71 } 71 }
72 72
73 // Generates a fixit hint to remove the 'virtual' keyword.
74 // Unfortunately, there doesn't seem to be a good way to determine the source
75 // location of the 'virtual' keyword. It's available in Declarator, but that
76 // isn't accessible from the AST. So instead, make an educated guess that the
77 // first token is probably the virtual keyword. Strictly speaking, this doesn't
78 // have to be true, but it probably will be.
79 // TODO(dcheng): Add a warning to force virtual to always appear first ;-)
73 FixItHint FixItRemovalForVirtual(const SourceManager& manager, 80 FixItHint FixItRemovalForVirtual(const SourceManager& manager,
74 const CXXMethodDecl* method) { 81 const CXXMethodDecl* method) {
75 // Unfortunately, there doesn't seem to be a good way to determine the
76 // location of the 'virtual' keyword. It's available in Declarator, but that
77 // isn't accessible from the AST. So instead, make an educated guess that the
78 // first token is probably the virtual keyword. Strictly speaking, this
79 // doesn't have to be true, but it probably will be.
80 // TODO(dcheng): Add a warning to force virtual to always appear first ;-)
81 SourceRange range(method->getLocStart()); 82 SourceRange range(method->getLocStart());
82 // Get the spelling loc just in case it was expanded from a macro. 83 // Get the spelling loc just in case it was expanded from a macro.
83 SourceRange spelling_range(manager.getSpellingLoc(range.getBegin())); 84 SourceRange spelling_range(manager.getSpellingLoc(range.getBegin()));
84 // Sanity check that the text looks like virtual. 85 // Sanity check that the text looks like virtual.
85 StringRef text = clang::Lexer::getSourceText( 86 StringRef text = clang::Lexer::getSourceText(
86 CharSourceRange::getTokenRange(spelling_range), manager, LangOptions()); 87 CharSourceRange::getTokenRange(spelling_range), manager, LangOptions());
87 if (text.trim() != "virtual") 88 if (text.trim() != "virtual")
88 return FixItHint(); 89 return FixItHint();
89 return FixItHint::CreateRemoval(range); 90 return FixItHint::CreateRemoval(range);
90 } 91 }
(...skipping 315 matching lines...) Expand 10 before | Expand all | Expand 10 after
406 OverrideAttr* override_attr = method->getAttr<OverrideAttr>(); 407 OverrideAttr* override_attr = method->getAttr<OverrideAttr>();
407 FinalAttr* final_attr = method->getAttr<FinalAttr>(); 408 FinalAttr* final_attr = method->getAttr<FinalAttr>();
408 409
409 if (IsMethodInBannedOrTestingNamespace(method)) 410 if (IsMethodInBannedOrTestingNamespace(method))
410 return; 411 return;
411 412
412 SourceManager& manager = instance().getSourceManager(); 413 SourceManager& manager = instance().getSourceManager();
413 414
414 // Complain if a method is annotated virtual && (override || final). 415 // Complain if a method is annotated virtual && (override || final).
415 if (has_virtual && (override_attr || final_attr)) { 416 if (has_virtual && (override_attr || final_attr)) {
416 diagnostic().Report(method->getLocStart(), 417 // ... but only if virtual does not originate in a macro from a banned file.
417 diag_redundant_virtual_specifier_) 418 // Note this is just an educated guess: the assumption here is that any
418 << "'virtual'" 419 // macro for declaring methods will probably be at the start of the method's
419 << (override_attr ? static_cast<Attr*>(override_attr) : final_attr) 420 // source range.
420 << FixItRemovalForVirtual(manager, method); 421 if (!InBannedDirectory(manager.getSpellingLoc(method->getLocStart()))) {
422 diagnostic().Report(method->getLocStart(),
423 diag_redundant_virtual_specifier_)
424 << "'virtual'"
425 << (override_attr ? static_cast<Attr*>(override_attr) : final_attr)
426 << FixItRemovalForVirtual(manager, method);
427 }
421 } 428 }
422 429
423 // Complain if a method is an override and is not annotated with override or 430 // Complain if a method is an override and is not annotated with override or
424 // final. 431 // final.
425 if (is_override && !override_attr && !final_attr) { 432 if (is_override && !override_attr && !final_attr) {
426 SourceRange type_info_range = 433 SourceRange range = method->getSourceRange();
427 method->getTypeSourceInfo()->getTypeLoc().getSourceRange(); 434 SourceLocation loc;
428 FullSourceLoc loc(type_info_range.getBegin(), manager); 435 if (method->hasInlineBody()) {
429 436 loc = method->getBody()->getSourceRange().getBegin();
430 // Build the FixIt insertion point after the end of the method definition,
431 // including any const-qualifiers and attributes, and before the opening
432 // of the l-curly-brace (if inline) or the semi-color (if a declaration).
433 SourceLocation spelling_end =
434 manager.getSpellingLoc(type_info_range.getEnd());
435 if (spelling_end.isValid()) {
436 SourceLocation token_end =
437 Lexer::getLocForEndOfToken(spelling_end, 0, manager, LangOptions());
438 diagnostic().Report(token_end, diag_method_requires_override_)
439 << FixItHint::CreateInsertion(token_end, " override");
440 } else { 437 } else {
441 diagnostic().Report(loc, diag_method_requires_override_); 438 // TODO(dcheng): We should probably use ASTContext's LangOptions here.
439 LangOptions lang_options;
440 loc = Lexer::getLocForEndOfToken(
441 manager.getSpellingLoc(range.getEnd()), 0,
442 manager, lang_options);
443 // The original code used the ending source loc of TypeSourceInfo's
444 // TypeLoc. Unfortunately, this breaks down in the presence of attributes.
445 // Attributes often appear at the end of a TypeLoc, e.g.
446 // virtual ULONG __stdcall AddRef()
447 // has a TypeSourceInfo that looks something like:
448 // ULONG AddRef() __attribute(stdcall)
449 // so a fix-it insertion would be generated to insert 'override' after
450 // __stdcall in the code as written.
451 // While using the spelling loc of the CXXMethodDecl fixes attribute
452 // handling, it breaks handling of "= 0" and similar constructs.. To work
453 // around this, scan backwards in the source text for a '=' or ')' token
454 // and adjust the location as needed...
455 for (SourceLocation l = loc.getLocWithOffset(-1);
456 l != manager.getLocForStartOfFile(manager.getFileID(loc));
457 l = l.getLocWithOffset(-1)) {
458 l = Lexer::GetBeginningOfToken(l, manager, lang_options);
459 Token token;
460 // getRawToken() returns *true* on failure. In that case, just give up
461 // and don't bother generating a possibly incorrect fix-it.
462 if (Lexer::getRawToken(l, token, manager, lang_options, true)) {
463 loc = SourceLocation();
464 break;
465 }
466 if (token.is(tok::r_paren)) {
467 break;
468 } else if (token.is(tok::equal)) {
469 loc = l;
470 break;
471 }
472 }
473 }
474 if (loc.isValid()) {
475 diagnostic().Report(loc, diag_method_requires_override_)
476 << FixItHint::CreateInsertion(loc, " override");
477 } else {
478 diagnostic().Report(range.getBegin(), diag_method_requires_override_);
442 } 479 }
443 } 480 }
444 481
445 if (final_attr && override_attr) { 482 if (final_attr && override_attr) {
446 diagnostic().Report(override_attr->getLocation(), 483 diagnostic().Report(override_attr->getLocation(),
447 diag_redundant_virtual_specifier_) 484 diag_redundant_virtual_specifier_)
448 << override_attr << final_attr 485 << override_attr << final_attr
449 << FixItHint::CreateRemoval(override_attr->getRange()); 486 << FixItHint::CreateRemoval(override_attr->getRange());
450 } 487 }
451 488
(...skipping 100 matching lines...) Expand 10 before | Expand all | Expand 10 after
552 return PublicDestructor; 589 return PublicDestructor;
553 } 590 }
554 } 591 }
555 592
556 return None; 593 return None;
557 } 594 }
558 595
559 // Adds either a warning or error, based on the current handling of 596 // Adds either a warning or error, based on the current handling of
560 // -Werror. 597 // -Werror.
561 DiagnosticsEngine::Level FindBadConstructsConsumer::getErrorLevel() { 598 DiagnosticsEngine::Level FindBadConstructsConsumer::getErrorLevel() {
599 #if defined(LLVM_ON_WIN32)
600 // TODO(dcheng): Re-enable -Werror for these diagnostics on Windows once all
601 // the pre-existing warnings are cleaned up. https://crbug.com/467287
602 return DiagnosticsEngine::Warning;
603 #else
562 return diagnostic().getWarningsAsErrors() ? DiagnosticsEngine::Error 604 return diagnostic().getWarningsAsErrors() ? DiagnosticsEngine::Error
563 : DiagnosticsEngine::Warning; 605 : DiagnosticsEngine::Warning;
606 #endif
564 } 607 }
565 608
566 // Returns true if |base| specifies one of the Chromium reference counted 609 // Returns true if |base| specifies one of the Chromium reference counted
567 // classes (base::RefCounted / base::RefCountedThreadSafe). 610 // classes (base::RefCounted / base::RefCountedThreadSafe).
568 bool FindBadConstructsConsumer::IsRefCountedCallback( 611 bool FindBadConstructsConsumer::IsRefCountedCallback(
569 const CXXBaseSpecifier* base, 612 const CXXBaseSpecifier* base,
570 CXXBasePath& path, 613 CXXBasePath& path,
571 void* user_data) { 614 void* user_data) {
572 FindBadConstructsConsumer* self = 615 FindBadConstructsConsumer* self =
573 static_cast<FindBadConstructsConsumer*>(user_data); 616 static_cast<FindBadConstructsConsumer*>(user_data);
(...skipping 213 matching lines...) Expand 10 before | Expand all | Expand 10 after
787 // one of those, it means there is at least one member after a factory. 830 // one of those, it means there is at least one member after a factory.
788 if (weak_ptr_factory_location.isValid() && 831 if (weak_ptr_factory_location.isValid() &&
789 !param_is_weak_ptr_factory_to_self) { 832 !param_is_weak_ptr_factory_to_self) {
790 diagnostic().Report(weak_ptr_factory_location, 833 diagnostic().Report(weak_ptr_factory_location,
791 diag_weak_ptr_factory_order_); 834 diag_weak_ptr_factory_order_);
792 } 835 }
793 } 836 }
794 } 837 }
795 838
796 } // namespace chrome_checker 839 } // namespace chrome_checker
OLDNEW
« no previous file with comments | « tools/clang/plugins/ChromeClassTester.cpp ('k') | tools/clang/plugins/tests/overridden_methods.txt » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698