Changeset 233054 in webkit
- Timestamp:
- Jun 21, 2018 1:10:15 PM (6 years ago)
- Location:
- trunk/Tools
- Files:
-
- 3 edited
-
ChangeLog (modified) (1 diff)
-
Scripts/webkitpy/style/checkers/cpp.py (modified) (8 diffs)
-
Scripts/webkitpy/style/checkers/cpp_unittest.py (modified) (2 diffs)
Legend:
- Unmodified
- Added
- Removed
-
trunk/Tools/ChangeLog
r233050 r233054 1 2018-06-21 Keith Rollin <krollin@apple.com> 2 3 check-webkit-style should warn about exported inline functions 4 https://bugs.webkit.org/show_bug.cgi?id=186861 5 <rdar://problem/41303668> 6 7 Reviewed by Brent Fulgham. 8 9 When checking binaries compiled with LTO enabled, WebKit's 10 check-for-weak-vtables-and-externals script can complain about 11 exported inline functions. For instance, in 12 Source/WebCore/page/scrolling/ScrollingTree.h, the following: 13 14 WEBCORE_EXPORT virtual void reportSynchronousScrollingReasonsChanged(MonotonicTime, SynchronousScrollingReasons) { } 15 WEBCORE_EXPORT virtual void reportExposedUnfilledArea(MonotonicTime, unsigned /* unfilledArea */) { } 16 17 Can result in the following error messages: 18 19 ERROR: WebCore has a weak external symbol in it (.../OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore) 20 ERROR: A weak external symbol is generated when a symbol is defined in multiple compilation units and is also marked as being exported from the library. 21 ERROR: A common cause of weak external symbols is when an inline function is listed in the linker export file. 22 ERROR: symbol __ZN7WebCore13ScrollingTree25reportExposedUnfilledAreaEN3WTF13MonotonicTimeEj 23 ERROR: symbol __ZN7WebCore13ScrollingTree40reportSynchronousScrollingReasonsChangedEN3WTF13MonotonicTimeEj 24 25 Unfortunately, these errors are only emitted when LTO is enabled, 26 meaning that a developer could check-in a file that will fail an LTO 27 build if they don't build with that option locally. Therefore, try to 28 head this off by updating check-webkit-style to identify and warn 29 about these cases (which includes when an export macro is applied 30 directly to an inline method as well as when an inline method is part 31 of an exported class). 32 33 * Scripts/webkitpy/style/checkers/cpp.py: 34 (_FunctionState.begin): 35 (_FunctionState.export_macro): 36 (_ClassInfo.__init__): 37 (check_for_non_standard_constructs): 38 (check_function_definition): 39 (process_line): 40 (CppChecker): 41 * Scripts/webkitpy/style/checkers/cpp_unittest.py: 42 (FunctionDetectionTest.perform_function_detection): 43 (FunctionDetectionTest.test_webcore_export): 44 1 45 2018-06-21 Aakash Jain <aakash_jain@apple.com> 2 46 -
trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py
r232986 r233054 530 530 self.parameter_start_position = parameter_start_position 531 531 self.parameter_end_position = parameter_end_position 532 self.is_final = False533 self.is_override = False534 self.is_pure = False535 532 characters_after_parameters = SingleLineView(clean_lines.elided, parameter_end_position, body_start_position).single_line 536 533 self.is_final = bool(search(r'\bfinal\b', characters_after_parameters)) … … 550 547 def is_virtual(self): 551 548 return bool(search(r'\bvirtual\b', self.modifiers_and_return_type())) 549 550 def export_macro(self): 551 export_match = match( 552 r'\b(WTF_EXPORT|WTF_EXPORT_PRIVATE|PAL_EXPORT|JS_EXPORT_PRIVATE|WEBCORE_EXPORT)\b', self.modifiers_and_return_type()) 553 return export_match.group(1) if export_match else None 552 554 553 555 def parameter_list(self): … … 1087 1089 """Stores information about a class.""" 1088 1090 1089 def __init__(self, name, line_number ):1091 def __init__(self, name, line_number, export_macro): 1090 1092 self.name = name 1091 1093 self.line_number = line_number 1094 self.export_macro = export_macro 1092 1095 self.seen_open_brace = False 1093 1096 self.is_derived = False … … 1358 1361 # Look for a class declaration 1359 1362 class_decl_match = match( 1360 r'\s*(template\s*<[\w\s<>,:]*>\s*)?(class|struct) \s+(\w+(::\w+)*)', line)1363 r'\s*(template\s*<[\w\s<>,:]*>\s*)?(class|struct)(\s+(WTF_EXPORT|WTF_EXPORT_PRIVATE|PAL_EXPORT|JS_EXPORT_PRIVATE|WEBCORE_EXPORT))?\s+(\w+(::\w+)*)', line) 1361 1364 if class_decl_match: 1362 classinfo_stack.append(_ClassInfo(class_decl_match.group( 3), line_number))1365 classinfo_stack.append(_ClassInfo(class_decl_match.group(5), line_number, class_decl_match.group(4))) 1363 1366 1364 1367 # Everything else in this function uses the top of the stack if it's … … 1660 1663 1661 1664 1662 def check_function_definition(filename, file_extension, clean_lines, line_number, function_state, error):1665 def check_function_definition(filename, file_extension, clean_lines, line_number, class_state, function_state, error): 1663 1666 """Check that function definitions for style issues. 1664 1667 … … 1701 1704 if function_state.is_override and function_state.is_final: 1702 1705 _error_redundant_specifier(line_number, 'override', 'final', error) 1706 1707 if not function_state.is_declaration: 1708 if (function_state.export_macro() 1709 and not function_state.export_macro() == 'JS_EXPORT_PRIVATE'): 1710 error(line_number, 'build/webcore_export', 4, 1711 'Inline functions should not be annotated with %s. Remove the macro, or ' 1712 'move the inline function definition out-of-line.' % 1713 function_state.export_macro()) 1714 elif (class_state.classinfo_stack 1715 and class_state.classinfo_stack[-1].export_macro 1716 and not class_state.classinfo_stack[-1].export_macro == 'JS_EXPORT_PRIVATE'): 1717 error(line_number, 'build/webcore_export', 4, 1718 'Inline functions should not be in classes annotated with %s. Remove the ' 1719 'macro from the class and apply it to each appropriate method, or move ' 1720 'the inline function definition out-of-line.' % 1721 class_state.classinfo_stack[-1].export_macro) 1703 1722 1704 1723 … … 3823 3842 if asm_state.is_in_asm(): # Ignore further checks because asm blocks formatted differently. 3824 3843 return 3825 check_function_definition(filename, file_extension, clean_lines, line, function_state, error)3844 check_function_definition(filename, file_extension, clean_lines, line, class_state, function_state, error) 3826 3845 check_for_leaky_patterns(clean_lines, line, function_state, error) 3827 3846 check_for_multiline_comments_and_strings(clean_lines, line, error) … … 3919 3938 'build/using_namespace', 3920 3939 'build/cpp_comment', 3940 'build/webcore_export', 3921 3941 'build/wk_api_available', 3922 3942 'legal/copyright', -
trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py
r232986 r233054 387 387 self.assertEqual(function_state.is_virtual(), function_information['is_virtual']) 388 388 self.assertEqual(function_state.is_declaration, function_information['is_declaration']) 389 self.assertEqual(function_state.export_macro(), function_information['export_macro'] if 'export_macro' in function_information else None) 389 390 self.assert_positions_equal(function_state.function_name_start_position, function_information['function_name_start_position']) 390 391 self.assert_positions_equal(function_state.parameter_start_position, function_information['parameter_start_position']) … … 624 625 'is_pure': False, 625 626 'is_declaration': True}) 627 628 def test_webcore_export(self): 629 self.perform_function_detection( 630 ['void theTestFunctionName();'], 631 {'name': 'theTestFunctionName', 632 'modifiers_and_return_type': 'void', 633 'function_name_start_position': (0, 5), 634 'parameter_start_position': (0, 24), 635 'parameter_end_position': (0, 26), 636 'body_start_position': (0, 26), 637 'end_position': (0, 27), 638 'is_final': False, 639 'is_override': False, 640 'is_virtual': False, 641 'is_pure': False, 642 'is_declaration': True, 643 'export_macro': None}) 644 645 self.perform_function_detection( 646 ['WEBCORE_EXPORT void theTestFunctionName();'], 647 {'name': 'theTestFunctionName', 648 'modifiers_and_return_type': 'WEBCORE_EXPORT void', 649 'function_name_start_position': (0, 20), 650 'parameter_start_position': (0, 39), 651 'parameter_end_position': (0, 41), 652 'body_start_position': (0, 41), 653 'end_position': (0, 42), 654 'is_final': False, 655 'is_override': False, 656 'is_virtual': False, 657 'is_pure': False, 658 'is_declaration': True, 659 'export_macro': 'WEBCORE_EXPORT'}) 660 661 self.perform_function_detection( 662 ['void theTestFunctionName()', 663 '{', 664 '}'], 665 {'name': 'theTestFunctionName', 666 'modifiers_and_return_type': 'void', 667 'function_name_start_position': (0, 5), 668 'parameter_start_position': (0, 24), 669 'parameter_end_position': (0, 26), 670 'body_start_position': (1, 0), 671 'end_position': (2, 1), 672 'is_final': False, 673 'is_override': False, 674 'is_virtual': False, 675 'is_pure': False, 676 'is_declaration': False, 677 'export_macro': None}) 678 679 self.perform_function_detection( 680 ['WEBCORE_EXPORT void theTestFunctionName()', 681 '{', 682 '}'], 683 {'name': 'theTestFunctionName', 684 'modifiers_and_return_type': 'WEBCORE_EXPORT void', 685 'function_name_start_position': (0, 20), 686 'parameter_start_position': (0, 39), 687 'parameter_end_position': (0, 41), 688 'body_start_position': (1, 0), 689 'end_position': (2, 1), 690 'is_final': False, 691 'is_override': False, 692 'is_virtual': False, 693 'is_pure': False, 694 'is_declaration': False, 695 'export_macro': 'WEBCORE_EXPORT'}) 626 696 627 697 def test_ignore_macros(self):
Note: See TracChangeset
for help on using the changeset viewer.
