Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compile errors .... chromium-style #2680

Closed
doctord919 opened this issue Mar 20, 2021 · 1 comment · Fixed by #2561 or #2673
Closed

compile errors .... chromium-style #2680

doctord919 opened this issue Mar 20, 2021 · 1 comment · Fixed by #2561 or #2673
Assignees
Labels
kind: bug release item: 🔨 further change solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@doctord919
Copy link

doctord919 commented Mar 20, 2021

I use OneFile ----> single_include/nlohmann/json.hpp
I use nlohmann::json 3.9.1. I compiled it on Windows 10Pro 64bit
I'm making a program that is based on the chrome source code. I do:
mytest.cc:
#include "json.hpp"
// for convenience
using json = nlohmann::json;

When I compile mytest.cc I get errors:
1 error:
json.hpp(2645,5): error: [chromium-style] virtual methods with non-empty bodies shouldn't be declared inline.
{
^
2,3,4 errors:
../../brave/vendor/bat-native-ads/src/bat/ads/internal/mytest/json.hpp(16417,9): error: [chromium-style] auto variable type must not deduce to a raw pointer type.
auto buffer_ptr = number_buffer.begin();
^~~~
auto*
../../brave/vendor/bat-native-ads/src/bat/ads/internal/mytest/json.hpp(16522,13): error: [chromium-style] auto variable type must not deduce to a raw pointer type.
const auto end = std::remove(number_buffer.begin(),
^~~~~~~~~~
auto* const
../../brave/vendor/bat-native-ads/src/bat/ads/internal/mytest/json.hpp(16532,13): error: [chromium-style] auto variable type must not deduce to a raw pointer type.
const auto dec_pos = std::find(number_buffer.begin(), number_buffer.end(), decimal_point);
^~~~~~~~~~
auto* const
4 errors generated.
2,3,4 solved:
json.hpp(16417,9)
- auto buffer_ptr = number_buffer.begin();
+ auto* buffer_ptr = number_buffer.begin();
json.hpp(16522,13)
- const auto end = std::remove(number_buffer.begin(),
+ auto* const end = std::remove(number_buffer.begin(),
json.hpp(16532,13)
- const auto dec_pos = std::find(number_buffer.begin(), number_buffer.end(), decimal_point);
+ auto* const dec_pos = std::find(number_buffer.begin(), number_buffer.end(), decimal_point);

But 1 error not solved...
I found the following information ...
https://www.chromium.org/developers/coding-style/chromium-style-checker-errors
`
Virtual Method Out-of-lining
Virtual methods are almost never inlined in practice. Because of this, methods on a base class will be emitted in each translation unit that allocates the object or any subclasses that don't override that method. This is usually not a major gain, unless you have a wide class hierarchy in which case it can be a big win (http://codereview.chromium.org/5741001/). Since virtual methods will almost never be inlined anyway (because they'll need to go through vtable dispatch), there are almost no downsides.

If you get the error:

virtual methods with non-empty bodies shouldn't be declared inline

It's because you wrote something like this:

class BaseClass {
 public:
  virtual void ThisOneIsOK() {}
  virtual bool ButWeDrawTheLineAtMethodsWithRealImplementation() { return false; }
};

And can be fixed by out of lining the method that does work:

class BaseClass {
 public:
  virtual void ThisIsHandledByTheCompiler() {}
  virtual bool AndThisOneIsDefinedInTheImplementation();
};

..... I comment 2643-2648:
/* JSON_HEDLEY_RETURNS_NON_NULL
const char* what() const noexcept override
{
return m.what();
}
*/
`
and json compiled & test json example is work ... but the sediment remains

@nlohmann
Copy link
Owner

Some fixes were already made, see #2067 #2561. But I think a header-only library has no real way to fix the virtual methods with non-empty bodies shouldn't be declared inline warning.

This was linked to pull requests Mar 23, 2021
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Mar 23, 2021
@nlohmann nlohmann self-assigned this Mar 24, 2021
@nlohmann nlohmann added this to the Release 3.9.2 milestone Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug release item: 🔨 further change solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants