Re: Pull request 1213 in include-what-you-use: RFC: Improved suggestions for namespace use

1 view
Skip to first unread message

notifi...@include-what-you-use.org

unread,
May 18, 2023, 4:28:55 AM5/18/23
to include-wh...@googlegroups.com
Comment #10 on pull request 1213 by headhunter45: RFC: Improved suggestions for namespace use
https://github.com/include-what-you-use/include-what-you-use/pull/1213

I'm probably misunderstanding this, but if I have ```#include <string>
using namespace std;``` at the top of a file and I don't actually use std::string or anything else from the std namespace what will this do? I'd want it to remove both the include and the using statement, but I think having the using would keep the `#include <string>` in this case. and IWYU wouldn't remove the using because I guess that's out of scope for the tool?


notifi...@include-what-you-use.org

unread,
May 18, 2023, 4:50:30 AM5/18/23
to include-wh...@googlegroups.com
Comment #11 on pull request 1213 by kimgr: RFC: Improved suggestions for namespace use
https://github.com/include-what-you-use/include-what-you-use/pull/1213

@headhunter45 Yes, exactly. Otherwise it's hard to know when to stop - if you have an unused static function in your file that uses `std::string`, do we remove that? Etc.

IWYU's focus is the relationship between symbols and files, and I think it's nice to stay within those fences.

That said, it would be cool to see some clang-tidy checks to do e.g. unused `using` cleanup. Then IWYU would be free to remove the leftover headers.


notifi...@include-what-you-use.org

unread,
May 26, 2023, 11:14:12 AM5/26/23
to include-wh...@googlegroups.com
Comment #12 on pull request 1213 by kilroyd: RFC: Improved suggestions for namespace use
https://github.com/include-what-you-use/include-what-you-use/pull/1213

Finally got round to moving the logic into `IwyuFileInfo`. Let me know if you prefer different placement within the file.

Performance-wise, there's a small hit. Locally the `run_all_test.sh` runtime goes from ~20.2s to ~20.7s (including the new test).


Reply all
Reply to author
Forward
0 new messages