[1/3] abidiff.cc: tidy using directives and declarations
Commit Message
This patch replaces a using directive with a couple of specific using
declarations, moves a couple of globally-scoped using declarations to
the top of the file and alphabetically sorts all the using
declarations.
* tools/abidiff.cc: Eliminate using directive. Move stray
using declarations to top of file and sort all using
declarations.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
tools/abidiff.cc | 44 ++++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 22 deletions(-)
Comments
Giuliano Procida <gprocida@google.com> a écrit:
> This patch replaces a using directive with a couple of specific using
> declarations, moves a couple of globally-scoped using declarations to
> the top of the file and alphabetically sorts all the using
> declarations.
Are these kinds of patches really necessary? I mean, I am not sure why
this kind of sorting is useful. Besides, it adds unnecessary noise to
the history, making "git blame" less useful.
Just like for the #include patch earlier, I am not for enforcing
alphabetical sorting for of the using declarations accross the project.
Or am I missing something?
Cheers,
Hi.
On Wed, 13 May 2020 at 12:27, Dodji Seketeli <dodji@seketeli.org> wrote:
> Giuliano Procida <gprocida@google.com> a écrit:
>
> > This patch replaces a using directive with a couple of specific using
> > declarations, moves a couple of globally-scoped using declarations to
> > the top of the file and alphabetically sorts all the using
> > declarations.
>
> Are these kinds of patches really necessary? I mean, I am not sure why
> this kind of sorting is useful. Besides, it adds unnecessary noise to
> the history, making "git blame" less useful.
>
> Just like for the #include patch earlier, I am not for enforcing
> alphabetical sorting for of the using declarations accross the project.
>
> Or am I missing something?
>
Thanks for calling this out, it deserves some discussion. I should have put
more justification into the commit message.
I think this comes from a couple of things.
Firstly, it is part of an internal coding standard that has received
thousands of hours of review and has been tested on very large scale code
bases. We probably follow parts of it by habit. Not all of it makes sense
outside of Google (where there a lot more than the Standard Library is
available and we have linting tools that help enforce compliance), but a
lot does and quite a bit will be non-controversial. The external version is
https://google.github.io/styleguide/cppguide.html and includes rationales.
Secondly, I was asked to do this in a review comment. This is probably just
the point above, by proxy, but it could be Matthias following his own
internal or some other style guide. I think the right thing to do
(initially) is to push back and say "this is not Google" in the cases where
there's no benefit.
You may want to consider adopting or adapting some of the style guide,
where you feel it would be a good match for the project, and brings some
value (such as increased readability, safety or performance).
Do let me know if I should drop this change and fix up the remaining ones.
Thanks!
Regards,
Giuliano.
> Cheers,
>
> --
> Dodji
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kernel-team+unsubscribe@android.com.
>
>
Hello
Giuliano Procida <gprocida@google.com> a écrit:
[...]
> Do let me know if I should drop this change and fix up the remaining
> ones.
Yeah, sorry, I prefer we just drop it.
Cheers,
@@ -33,36 +33,39 @@
#include "abg-reader.h"
#include "abg-dwarf-reader.h"
-using std::vector;
-using std::string;
-using std::ostream;
-using std::cout;
-using std::cerr;
using abg_compat::shared_ptr;
-using abigail::ir::environment;
-using abigail::ir::environment_sptr;
-using abigail::translation_unit;
-using abigail::translation_unit_sptr;
-using abigail::corpus_sptr;
-using abigail::corpus_group_sptr;
-using abigail::comparison::translation_unit_diff_sptr;
+using abigail::comparison::compute_diff;
using abigail::comparison::corpus_diff;
using abigail::comparison::corpus_diff_sptr;
-using abigail::comparison::compute_diff;
-using abigail::comparison::get_default_harmless_categories_bitmap;
+using abigail::comparison::diff_context;
+using abigail::comparison::diff_context_sptr;
using abigail::comparison::get_default_harmful_categories_bitmap;
+using abigail::comparison::get_default_harmless_categories_bitmap;
+using abigail::comparison::translation_unit_diff_sptr;
+using abigail::corpus_group_sptr;
+using abigail::corpus_sptr;
+using abigail::dwarf_reader::STATUS_ALT_DEBUG_INFO_NOT_FOUND;
+using abigail::dwarf_reader::STATUS_DEBUG_INFO_NOT_FOUND;
+using abigail::ir::environment;
+using abigail::ir::environment_sptr;
+using abigail::suppr::read_suppressions;
using abigail::suppr::suppression_sptr;
using abigail::suppr::suppressions_type;
-using abigail::suppr::read_suppressions;
-using namespace abigail::dwarf_reader;
-using abigail::tools_utils::emit_prefix;
+using abigail::tools_utils::abidiff_status;
using abigail::tools_utils::check_file;
-using abigail::tools_utils::guess_file_type;
+using abigail::tools_utils::emit_prefix;
using abigail::tools_utils::gen_suppr_spec_from_headers;
using abigail::tools_utils::gen_suppr_spec_from_kernel_abi_whitelists;
+using abigail::tools_utils::guess_file_type;
using abigail::tools_utils::load_default_system_suppressions;
using abigail::tools_utils::load_default_user_suppressions;
-using abigail::tools_utils::abidiff_status;
+using abigail::translation_unit;
+using abigail::translation_unit_sptr;
+using std::cerr;
+using std::cout;
+using std::ostream;
+using std::string;
+using std::vector;
struct options
{
@@ -625,9 +628,6 @@ display_symtabs(const corpus_sptr c1, const corpus_sptr c2, ostream& o)
o << (*i)->get_pretty_representation() << std::endl;
}
-using abigail::comparison::diff_context_sptr;
-using abigail::comparison::diff_context;
-
/// Check that the suppression specification files supplied are
/// present. If not, emit an error on stderr.
///