{"push_id":2985889522,"size":18,"distinct_size":16,"ref":"refs/heads/eh/add_compiler_warnings","head":"82386e31321cf2e8141c66b1325745c878ba67bf","before":"39821459fa71ce37ef3f70e21a128bdba6562ae3","commits":[{"sha":"156aeea1a6fe4929b6f3a1b9a15b8d1a4f6e9f77","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"vec: remove `vec::Doubled`\n\nIt is now unused, so let's remove it!","distinct":false,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/156aeea1a6fe4929b6f3a1b9a15b8d1a4f6e9f77"},{"sha":"556c521813f3842e7b45f91707dc24d66f3b6b9b","author":{"name":"Sylvain Laperche","email":"37c8b4b371a6d024637b48045fe91dafb53d3171@users.noreply.github.com"},"message":"Merge pull request #259 from slaperche-scality/eh/remove_unused_vec_doubled\n\nRemove `vec::Doubled`","distinct":false,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/556c521813f3842e7b45f91707dc24d66f3b6b9b"},{"sha":"fa3c9a2a31f5c2f9e253e5fbf7e68647da64bdf7","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"fix `-Wignored-qualifiers`\n\nType qualifiers (such as const) are ignored on function return type.","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/fa3c9a2a31f5c2f9e253e5fbf7e68647da64bdf7"},{"sha":"44857deb06944d1396a78867b9338d3a936b7e43","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"fix `-Wsuggest-attribute=noreturn`\n\nAdd missing noreturn attribute.","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/44857deb06944d1396a78867b9338d3a936b7e43"},{"sha":"ee61467db73f5dfe6ddb276dd22660db15ed5e4d","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"add `narrow_cast`\n\n`narrow_cast` is a simple wrapper around `static_cast` and should be\nused when you cast to narrow a value.\n\nThat way, if one day we have a bug related to narrowing, we can simply\ngrep for the specific `narrow_cast` instead of the more general\n`static_cast`.","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/ee61467db73f5dfe6ddb276dd22660db15ed5e4d"},{"sha":"37ef8a7a6d269909c70505f1848420314293af1d","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"fix `-Wold-style-cast`\n\nDon't use C-like cast, C++ offers a wider range of cast, each own with\nits own semantic and rules.","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/37ef8a7a6d269909c70505f1848420314293af1d"},{"sha":"be7d874c7eaed85acccaaece5984af692a59b9e8","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"handle missing cases (flagged by `-Wswitch-enum`)","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/be7d874c7eaed85acccaaece5984af692a59b9e8"},{"sha":"9c852562865519093d1044c8815760b62fb20176","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"fix `-Wunused-parameter`\n\nNote the most of the occurences are fixed by replacing some pure virtual\nmethods by virtual methods with a default body that does nothing (as\nmost of the children classes were doing nothing…).\n\nAlso remove a truly unused parameter from `unpack`.","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/9c852562865519093d1044c8815760b62fb20176"},{"sha":"2806de532a0d34dee5dc6c51b1e9225a207ffb17","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"fix `-Wmissing-declaration`\n\nEither add missing prototypes or mark the function with static.","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/2806de532a0d34dee5dc6c51b1e9225a207ffb17"},{"sha":"b4daf215eb4789ba7fb9e87ae92debe05acbd6d0","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"fix `-Woverloaded-virtual`\n\nHere is a quick drill down, for those unfamiliar with this aspect of\nC++.\n\nWhat's is this about?\n=====================\n\nThe first thing to note is that there is no overloading across scopes,\nand derived class scopes are not an exception.\n\nConcretely, that means that this code:\n\n    struct A {\n        int foo(int i) { std::cout << \"foo(int): \"; return i+1; }\n    };\n\n    struct B : public A {\n        double f(double d) { std::cout << \"foo(double): \"; return d+1.3; }\n    };\n    […]\n    B* pd = new B;\n    std::cout << pb->f(2) << '\\n';\n\nWill produce:\n\n    f(double): 3.3\n\nBecause the best method is only looked up into D scope, so even though a\nbetter match is available for `int` in B, it's not found and the version\nfor `double` is used.\n\nThe compiler warning is triggered when the hidden function is virtual,\nas shown in the example from the man:\n\n    struct A {\n        virtual void f();\n    };\n\n    struct B: public A {\n        void f(int);\n    };\n\nthe A class version of f is hidden in B, and code like:\n\n    B* b;\n    b->f();\n\nfails to compile.\n\nWhat about us?\n==============\n\nOur codebase is affected by this problem, the hidden functions are:\n- void FecCode<T>::decode_apply(const DecodeContext<T>&, Buffers<T>&, Buffers<T>&)\n- void FecCode<T>::decode_prepare(const DecodeContext<T>&, const std::vector<Properties>&, off_t, Buffers<T>&)\n- void FecCode<T>::encode_post_process(Buffers<T>&, std::vector<Properties>&, off_t)\n- void FecCode<T>::encode(Buffers<T>&, std::vector<quadiron::Properties>&, off_t, Buffers<T>&)\n- void FourierTransform<T>::fft_inv(Buffers<T>&, Buffers<T>&)\n- void FourierTransform<T>::fft(Buffers<T>&, Buffers<T>&)\n- void FourierTransform<T>::ifft(Buffers<T>&, Buffers<T>&)\n- void RingModN<T>::neg(Buffers<T>&) const\n- void RingModN<T>::neg(size_t, T*) const\n\nYou can see that most of them are `Buffers` version of pure abtract\nmethods that works on `Vector`.\n\nSince the `Vector` versions are pure abstract, children classes must\noverride them and by doing so mask the overloaded versions that work on\n`Buffers` (except if they also override the `Buffers` versions`, but\nthat's not always the case).\n\nExample:\n- `fft::Radix2` override both `Vector` and `Buffers` versions\n  => no compiler warning.\n- `fft::CooleyTukey` doesn't override the `Buffers` versions\n  => we have a warning.\n\nBut our code compile/works!\n===========================\n\nYeah, by happenstance.\n\nBecause most of these `Buffers` version are defined as no-op by default\nand if a child class doesn't override it that propably means that it's\nnever called and thus yeah, our code compile.\n\nBut that doesn't change the fact that the code is brittle, and we should\nclean these up!\n\nHow do we fix this?\n===================\n\nBy explicitely asking for the methods lookup to consider the scope of\nthe parent class, which is done with an using-declaration.\n\nAppendix: why this weird and confusing behavior‽\n================================================\n\nBecause if we were looking up into the parents' scopes, then the code\nhave certain unforeseen and potentially dangerous behavior.\n\nConsider the following code:\n\n    class Base {\n        int x;\n    public:\n        virtual void copy(Base* p) { x = p-> x; }\n    };\n\n    class Derived{\n        int xx;\n    public:\n        virtual void copy(Derived* p) { xx = p->xx; Base::copy(p); }\n    };\n\n    void f(Base a, Derived b)\n    {\n        a.copy(&b); // ok: copy Base part of b\n        b.copy(&a); // BUG: Base::copy is called and `xx` is not copied!\n    }\n\nIf we were looking up into the parent's scope, then we would find\n`Base::copy(Base*)` which is a better match for the second call to copy\nBUT it would lead to object slicing, which is unexpected here!\n\nReferences\n==========\n\n- http://www.stroustrup.com/bs_faq2.html#overloadderived\n- https://stackoverflow.com/a/1629074","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/b4daf215eb4789ba7fb9e87ae92debe05acbd6d0"},{"sha":"054f5bc23cb9f3e4ea7abfe9a6db327545e1e39c","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"remove VLA (fix `-Wvla`).\n\nVLA are not supported in C++, that's a C99 feature not a C++ one (and\neven in C, it becames optional since C11).\n\nEven if they were supported, it's not a good idea to use them\n(especially with an unbounded size such as `this->n`) this is a recipe\nfor easy stack overflows.\n\nNow, dumbly replacing VLAs by `std::vector` simply won't do (the code\nbecomes 50% slower). But, we can implement a scratch space inside the\n`gf::NF4` object that can replaces the VLAs and get a nice 10% speedup :)","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/054f5bc23cb9f3e4ea7abfe9a6db327545e1e39c"},{"sha":"6d7d928363a0431eafcf15a81d91862eeff2c786","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"use typename instead of class\n\n`typename` is preferred in this context, and this is what we use in the\nrest of the codebase.","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/6d7d928363a0431eafcf15a81d91862eeff2c786"},{"sha":"ac39fa1b3c94f8bf859af8a199031f9722a30ff9","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"fix Doxygen documentation (flagged by `-Wdocumentation`)","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/ac39fa1b3c94f8bf859af8a199031f9722a30ff9"},{"sha":"75fb3dd948310e837e6734ae2d5b733ff231cdb0","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"follow the rule of five for vec::Vector\n\nSince we have a destructor, we add:\n- a copy constructor\n- a move constructor\n- a copy assignment operator\n- a move assignment operator\n\nClang was complaining about it (`-Wdeprecated`), and rightly so!\n\n    definition of implicit copy constructor for 'Vector<unsigned int>'\n    is deprecated because it has a user-declared destructor","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/75fb3dd948310e837e6734ae2d5b733ff231cdb0"},{"sha":"a57010c7c11fdf89f6150540bc5f1f5a660e18a9","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"fix `-Wshift-sign-overflow`","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/a57010c7c11fdf89f6150540bc5f1f5a660e18a9"},{"sha":"1937185770e986dea9550704c24199370573c564","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"add more compiler warnings\n\nThis will definitely help us spot dubious code and maintain a good level\nof code quality.\n\nNext step would be to enable more lint in clang-tidy.","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/1937185770e986dea9550704c24199370573c564"},{"sha":"804fd40a4f140439c4e729b88978c3a9d72521cc","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"ci: test build in debug mode with all compiler & all SIMD mode\n\nThe debug mode is the one with the more warnings, so this is the one\nthat should be tested (if it compiles in Debug, it will compile in\nRelease), but since we use conditionnal compilation for SIMD we should\nmake sure that every codepath is clean.\n\nCompilation in all SIMD mode in release is done by the test job anyway.","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/804fd40a4f140439c4e729b88978c3a9d72521cc"},{"sha":"82386e31321cf2e8141c66b1325745c878ba67bf","author":{"name":"Sylvain Laperche","email":"701d855556b86f2f79e552cf49b30b5d130ca6f0@scality.com"},"message":"build: fix SIMD flag handling\n\nWe should use list append, not string concat to add the SIMD flag.","distinct":true,"url":"https://api.github.com/repos/slaperche-scality/quadiron/commits/82386e31321cf2e8141c66b1325745c878ba67bf"}]}
{"actor":{"display_login":"slaperche-scality"}}