186796 2018-06-18 19:28:26 -0700 cpp style checker needs to know more about NOLINT 2018-06-19 14:58:38 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 krollin krollin aakash_jain commit-queue ddkilzer ews-watchlist glenn jbedard lforschler mcatanzaro webkit-bug-importer oldest_to_newest 1434270 0 krollin 2018-06-18 19:28:26 -0700 Running check-webkit-style on ThirdParty/gtest/src/gtest-internal-inl.h crashes with a traceback like: Traceback (most recent call last): File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 2830, in test_check_alphabetical_include_order '') File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 339, in assert_language_rules_check self.perform_language_rules_check(file_name, code, lines_to_check)) File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 287, in perform_language_rules_check return self.perform_lint(code, filename, basic_error_rules, lines_to_check=lines_to_check) File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 259, in perform_lint self.process_file_data(filename, extension, lines, error_collector, unit_test_config) File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 253, in process_file_data checker.check(lines) File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp.py", line 4030, in check self.handle_style_error, self.min_confidence) File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp.py", line 3884, in _process_lines enum_state, asm_state, error) File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp.py", line 3831, in process_line file_state, error) File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp.py", line 3122, in check_language check_include_line(filename, file_extension, clean_lines, line_number, include_state, error) File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp.py", line 3075, in check_include_line previous_header_type = include_state.header_types[previous_line_number] KeyError: 65 1434271 1 krollin 2018-06-18 19:29:01 -0700 The problem has to do with the following lines in gtest-internal-inl.h: #include <gtest/gtest.h> // NOLINT #include <gtest/gtest-spi.h> The NOLINT annotation will prevent the cpp-checker from processing or even seeing that line. In skipping that line, information regarding that line is NOT added to some internal tables. However, when the code that checks that #includes are correctly alphabetized gets to the "gtest-spi.h" line, it will not respect the NOLINT annotation and will see the gtest.h line. When doing so, it runs afoul of the fact that it hadn't seen that line before and it crashes when it tries to look up that line in some internal records. Fix this by catering to the possibility that a #include line may not have been entered into these internal records. 1434272 2 343009 krollin 2018-06-18 19:42:26 -0700 Created attachment 343009 Patch 1434503 3 webkit-bug-importer 2018-06-19 12:48:05 -0700 <rdar://problem/41260116> 1434560 4 jbedard 2018-06-19 14:14:46 -0700 Unofficial r=me. 1434584 5 343009 commit-queue 2018-06-19 14:58:37 -0700 Comment on attachment 343009 Patch Clearing flags on attachment: 343009 Committed r232986: <https://trac.webkit.org/changeset/232986> 1434585 6 commit-queue 2018-06-19 14:58:38 -0700 All reviewed patches have been landed. Closing bug. 343009 2018-06-18 19:42:26 -0700 2018-06-19 14:58:37 -0700 Patch bug-186796-20180618194226.patch text/plain 3741 krollin U3VidmVyc2lvbiBSZXZpc2lvbjogMjMyOTMzCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggYzY1Y2VmYjRlYTQwZGIxYTE3ZWRlN2VjNzhiYjhhNTM5 ZDQ2MjgwZS4uMjkyZWY1NWFhZmQ2ZmJkYzllOTRlZGZhMGFlOWQxMTBlMzc2YzIyYSAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDM1 IEBACisyMDE4LTA2LTE4ICBLZWl0aCBSb2xsaW4gIDxrcm9sbGluQGFwcGxlLmNvbT4KKworICAg ICAgICBjcHAgc3R5bGUgY2hlY2tlciBuZWVkcyB0byBrbm93IG1vcmUgYWJvdXQgTk9MSU5UCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODY3OTYKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBSdW5uaW5nIGNo ZWNrLXdlYmtpdC1zdHlsZSBvbgorICAgICAgICBUaGlyZFBhcnR5L2d0ZXN0L3NyYy9ndGVzdC1p bnRlcm5hbC1pbmwuaCBjcmFzaGVzIHdpdGggYSBLZXlFcnJvciBhcworICAgICAgICBpdCB0cmll cyB0byBsb29rIHVwIGluZm9ybWF0aW9uIGFib3V0IGEgcGFydGljdWxhciAjaW5jbHVkZSBpbiBh CisgICAgICAgIGRpY3Rpb25hcnkgaXQgbWFpbnRhaW5zLiBUaGUgcHJvYmxlbSBoYXMgdG8gZG8g d2l0aCB0aGUgZm9sbG93aW5nCisgICAgICAgIGxpbmVzIGluIGd0ZXN0LWludGVybmFsLWlubC5o OgorCisgICAgICAgICNpbmNsdWRlIDxndGVzdC9ndGVzdC5oPiAgLy8gTk9MSU5UCisgICAgICAg ICNpbmNsdWRlIDxndGVzdC9ndGVzdC1zcGkuaD4KKworICAgICAgICBUaGUgTk9MSU5UIGFubm90 YXRpb24gd2lsbCBwcmV2ZW50IHRoZSBjcHAtY2hlY2tlciBmcm9tIHByb2Nlc3Npbmcgb3IKKyAg ICAgICAgZXZlbiBzZWVpbmcgdGhhdCBsaW5lLiBJbiBza2lwcGluZyB0aGF0IGxpbmUsIGluZm9y bWF0aW9uIHJlZ2FyZGluZworICAgICAgICB0aGF0IGxpbmUgaXMgTk9UIGFkZGVkIHRvIHNvbWUg aW50ZXJuYWwgdGFibGVzLiBIb3dldmVyLCB3aGVuIHRoZSBjb2RlCisgICAgICAgIHRoYXQgY2hl Y2tzIHRoYXQgI2luY2x1ZGVzIGFyZSBjb3JyZWN0bHkgYWxwaGFiZXRpemVkIGdldHMgdG8gdGhl CisgICAgICAgICJndGVzdC1zcGkuaCIgbGluZSwgaXQgd2lsbCBub3QgcmVzcGVjdCB0aGUgTk9M SU5UIGFubm90YXRpb24gYW5kIHdpbGwKKyAgICAgICAgc2VlIHRoZSBndGVzdC5oIGxpbmUuIFdo ZW4gZG9pbmcgc28sIGl0IHJ1bnMgYWZvdWwgb2YgdGhlIGZhY3QgdGhhdCBpdAorICAgICAgICBo YWRuJ3Qgc2VlbiB0aGF0IGxpbmUgYmVmb3JlIGFuZCBpdCBjcmFzaGVzIHdoZW4gaXQgdHJpZXMg dG8gbG9vayB1cAorICAgICAgICB0aGF0IGxpbmUgaW4gc29tZSBpbnRlcm5hbCByZWNvcmRzLgor CisgICAgICAgIEZpeCB0aGlzIGJ5IGNhdGVyaW5nIHRvIHRoZSBwb3NzaWJpbGl0eSB0aGF0IGEg I2luY2x1ZGUgbGluZSBtYXkgbm90CisgICAgICAgIGhhdmUgYmVlbiBlbnRlcmVkIGludG8gdGhl c2UgaW50ZXJuYWwgcmVjb3Jkcy4KKworICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkvc3R5bGUv Y2hlY2tlcnMvY3BwLnB5OgorICAgICAgICAoY2hlY2tfaW5jbHVkZV9saW5lKToKKyAgICAgICAg KiBTY3JpcHRzL3dlYmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcF91bml0dGVzdC5weToKKwogMjAx OC0wNi0xOCAgWmFuIERvYmVyc2VrICA8emRvYmVyc2VrQGlnYWxpYS5jb20+CiAKICAgICAgICAg W3dlYmtpdHB5XSBXUFRSdW5uZXIgc2hvdWxkIHJlbW92ZSBhbnkgbWV0YWRhdGEgY29udGVudCBi ZWZvcmUgKHJlKWdlbmVyYXRpbmcgaXQKZGlmZiAtLWdpdCBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0 cHkvc3R5bGUvY2hlY2tlcnMvY3BwLnB5IGIvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9zdHlsZS9j aGVja2Vycy9jcHAucHkKaW5kZXggMzliMDhjYWQ0NjJjYzUwM2Y5YzliODRjMzIyMWM5YjkwZTU2 N2RiZi4uNTgzNmM0ZjQwMDhkMjkwZTI4NzE4NTIwNWUyMzMwMmIxMjYzMzgxNCAxMDA2NDQKLS0t IGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHAucHkKKysrIGIvVG9v bHMvU2NyaXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHAucHkKQEAgLTMwNzAsNyArMzA3 MCw3IEBAIGRlZiBjaGVja19pbmNsdWRlX2xpbmUoZmlsZW5hbWUsIGZpbGVfZXh0ZW5zaW9uLCBj bGVhbl9saW5lcywgbGluZV9udW1iZXIsIGluY2x1CiAgICAgICAgICAgICBwcmV2aW91c19saW5l X251bWJlciAtPSAxCiAgICAgICAgICAgICBwcmV2aW91c19saW5lID0gY2xlYW5fbGluZXMubGlu ZXNbcHJldmlvdXNfbGluZV9udW1iZXJdCiAgICAgICAgICAgICBwcmV2aW91c19tYXRjaCA9IF9S RV9QQVRURVJOX0lOQ0xVREUuc2VhcmNoKHByZXZpb3VzX2xpbmUpCi0gICAgICAgIGlmIHByZXZp b3VzX21hdGNoOgorICAgICAgICBpZiBwcmV2aW91c19tYXRjaCBhbmQgcHJldmlvdXNfbGluZV9u dW1iZXIgaW4gaW5jbHVkZV9zdGF0ZS5oZWFkZXJfdHlwZXM6CiAgICAgICAgICAgICBwcmV2aW91 c19oZWFkZXJfdHlwZSA9IGluY2x1ZGVfc3RhdGUuaGVhZGVyX3R5cGVzW3ByZXZpb3VzX2xpbmVf bnVtYmVyXQogICAgICAgICAgICAgaWYgcHJldmlvdXNfaGVhZGVyX3R5cGUgPT0gX09USEVSX0hF QURFUjoKICAgICAgICAgICAgICAgICBpZiAnPCcgaW4gcHJldmlvdXNfbGluZSBhbmQgJyInIGlu IGxpbmU6CmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3N0eWxlL2NoZWNrZXJz L2NwcF91bml0dGVzdC5weSBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvc3R5bGUvY2hlY2tlcnMv Y3BwX3VuaXR0ZXN0LnB5CmluZGV4IDc4MmRkN2UzZmIzMmEwYzFlZTU0Yzk4MmY0N2M0YjVhMjg5 MzU0Y2EuLmM1MDFhNDJiNDk2OTBmNDk2ZjJmMWMzNTVkY2UxZmM1ZjJhNDE2OTAgMTAwNjQ0Ci0t LSBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvc3R5bGUvY2hlY2tlcnMvY3BwX3VuaXR0ZXN0LnB5 CisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvc3R5bGUvY2hlY2tlcnMvY3BwX3VuaXR0ZXN0 LnB5CkBAIC0yODI0LDYgKzI4MjQsMTEgQEAgY2xhc3MgT3JkZXJPZkluY2x1ZGVzVGVzdChDcHBT dHlsZVRlc3RCYXNlKToKICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg JyNpbmNsdWRlICJhcnJheS5sdXQuaCJcbicsCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICcnKQogCisgICAgICAgIHNlbGYuYXNzZXJ0X2xhbmd1YWdlX3J1bGVzX2No ZWNrKCdmb28uaCcsCisgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICcj aW5jbHVkZSA8Z3Rlc3QvZ3Rlc3QuaD4gLy8gTk9MSU5UXG4nCisgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICcjaW5jbHVkZSA8Z3Rlc3QvZ3Rlc3Qtc3BpLmg+XG4nLAor ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAnJykKKwogICAgIGRlZiB0 ZXN0X2NoZWNrX2FscGhhYmV0aWNhbF9pbmNsdWRlX29yZGVyX2Vycm9yc19yZXBvcnRlZF9mb3Jf Ym90aF9saW5lcyhzZWxmKToKICAgICAgICAgIyBJZiBvbmUgb2YgdGhlIHR3byBsaW5lcyBvZiBv dXQgb2Ygb3JkZXIgaGVhZGVycyBhcmUgZmlsdGVyZWQsIHRoZSBlcnJvciBzaG91bGQgYmUKICAg ICAgICAgIyByZXBvcnRlZCBvbiB0aGUgb3RoZXIgbGluZS4K