Bug 190714 – Make generated C++ code use modern C++

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Rob Buis]

Make generated C++ code use modern C++

[Rob Buis]

Created attachment 352690 [details]
Patch

[Don Olmstead]

Nice start! Might be worth it to run some of the generated C++ through clang-tidy's modernizers and see if anything else pops out while you're at it.

[Rob Buis]

Hi Don,

(In reply to Don Olmstead from comment #2)
> Nice start! Might be worth it to run some of the generated C++ through
> clang-tidy's modernizers and see if anything else pops out while you're at
> it.

This does not work on OS X though, right? If so I'll try to get it to run next week when I should have access to linux again.

[Rob Buis]

Created attachment 352788 [details]
Patch

[Rob Buis]

Hi Don,

(In reply to Rob Buis from comment #3)
> Hi Don,
> 
> (In reply to Don Olmstead from comment #2)
> > Nice start! Might be worth it to run some of the generated C++ through
> > clang-tidy's modernizers and see if anything else pops out while you're at
> > it.
> 
> This does not work on OS X though, right? If so I'll try to get it to run
> next week when I should have access to linux again.

I now have clang-tidy's modernizers working on linux. However I don't think this checks  the generated c++ code, I think because they are not included in compile_commands.json. Did you ever run into this?

[Rob Buis]

Created attachment 400702 [details]
Patch

[EWS Watchlist]

This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)

[Blaze Burg]

Comment on attachment 400702 [details]
Patch

Looks good so far, let me know if you'd like feedback. Please ensure you include rebaselined tests when your patch is up for review.

[Rob Buis]

Thanks Brian! Will do, can probably update the patch later in the week.

[Rob Buis]

(In reply to Don Olmstead from comment #2)
> Nice start! Might be worth it to run some of the generated C++ through
> clang-tidy's modernizers and see if anything else pops out while you're at
> it.

Hi Don, I rebased the patch. Given your recent work on cmake + clang-tidy, do you think above is on your TODO list?

[Rob Buis]

Created attachment 400791 [details]
Patch

[Rob Buis]

(In reply to Brian Burg from comment #8)
> Comment on attachment 400702 [details]
> Patch
> 
> Looks good so far, let me know if you'd like feedback. Please ensure you
> include rebaselined tests when your patch is up for review.

I did include the rebaselined tests in the latest patch.

[EWS]

Committed r262424: <https://trac.webkit.org/changeset/262424>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400791 [details].

[Radar WebKit Bug Importer]

<rdar://problem/63877449>

[Jonathan Bedard]

This caused a webkitpy test regression.

https://results.webkit.org/?suite=webkitpy-tests&test=webkit.messages_unittest.HeaderTest.test_receiver_headers

(Internal testing narrowed the regression range to exactly this commit)

[Ryan Haddad]

Reverted r262424 for reason:

Caused webkitpy test failure

Committed r262461: <https://trac.webkit.org/changeset/262461>

[Rob Buis]

Created attachment 400899 [details]
Patch

[Rob Buis]

(In reply to Jonathan Bedard from comment #15)
> This caused a webkitpy test regression.
> 
> https://results.webkit.org/?suite=webkitpy-tests&test=webkit.
> messages_unittest.HeaderTest.test_receiver_headers
> 
> (Internal testing narrowed the regression range to exactly this commit)

webkitpy now runs without failure again. Let me know if anything more is needed.

[Jonathan Bedard]

Comment on attachment 400899 [details]
Patch

Let's try this again! :)

[EWS]

Committed r262486: <https://trac.webkit.org/changeset/262486>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400899 [details].