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

Many warnings [...] hides overloaded virtual function #155

Closed
jpcima opened this issue Jun 13, 2020 · 8 comments
Closed

Many warnings [...] hides overloaded virtual function #155

jpcima opened this issue Jun 13, 2020 · 8 comments

Comments

@jpcima
Copy link
Contributor

jpcima commented Jun 13, 2020

Hello, the file label.hpp emits a series of warnings of the same kind.

Since it's in the header, it propagates into the program which uses the library, it's a warning which I prefer to not disable on program side if possible.


Output is as follows.

/home/jpc/documents/projects/elements/lib/include/elements/element/label.hpp:132:31: warning: 'cycfi::elements::label_gen<cycfi::elements::basic_label_base<cycfi::elements::default_label> >::font' hides overloaded virtual function [-Woverloaded-virtual]
      gen_font                font(font_type font_) const;
                              ^

Compiler

clang version 10.0.0
Target: x86_64-pc-linux-gnu
@Xeverous
Copy link
Contributor

I was fixing similar warnings/problems already (see #29) but this one is non-trivial as the wrapping factory generator has more complex logic and AFAIK by design it intentionally wraps some of the label functions with different return types, inheritance is causing problems in this case. @djowel wrote these factories and maybe has an idea how they could be written differently to be more warning-safe.

@djowel I think this is a good place to start thinking about more functional-style programming interface, eg label("text") | font_size(14). I see this as a place for vast syntax improvements - the GUI in my own project starts to fall into a mess of multi-capturing lambdas.

@jpcima
Copy link
Contributor Author

jpcima commented Jun 14, 2020

Until this problem gets fixed, can this warning possibly be silenced with gcc diagnostic pragmas? If it could be replaced with a pragma message on .cpp side, this will be nowhere as annoying for the library user.

@Xeverous
Copy link
Contributor

This is a template code. Can not really move it to a source file. I would rather rewrite the wrapper to have a different interface.

@djowel
Copy link
Member

djowel commented Jun 14, 2020

I'll look into this asap.

@djowel
Copy link
Member

djowel commented Jun 14, 2020

@djowel I think this is a good place to start thinking about more functional-style programming interface, eg label("text") | font_size(14). I see this as a place for vast syntax improvements - the GUI in my own project starts to fall into a mess of multi-capturing lambdas.

I think the a | b syntax should be limited to the proxies. The example I showed illustrates why:

box | align.top.left | margin(10, 10, 20, 20) 

(see #145)

In the expression above, I still use the dot notation for "attributes" of similar kinds as it makes sense to group them together (i.e. align.top.left), otherwise, I fear that the expression template will be unmanageable. font, text_align and friends are "attributes" and as such, I'd like to keep the dot notation for those. The "functional-style" notation has one undesirable effect of populating the elements namespace with small generators, and that's what I want to minimize. Imagine the alternative:

box | align | top | left | margin(10, 10, 20, 20) 

@djowel
Copy link
Member

djowel commented Jun 24, 2020

This is now fixed in the warnings-as-errors branch, along with lots of cleanup.

@djowel djowel closed this as completed Jun 24, 2020
@jpcima
Copy link
Contributor Author

jpcima commented Jul 1, 2020

The branch warnings-as-errors resolves the problem, but creates another also. (rather trivial)
If I may ask, can we enable Werror on demand only? this sometimes gives software packagers trouble, when they are using different compiler versions.

/home/jpc/documents/projects/elements/lib/host/linux/window.cpp:52:23: error: lambda capture 'style_' is not used [-Werror,-Wunused-lambda-capture]
         [this, name, style_, bounds]()
                    ~~^~~~~~
1 error generated.

@djowel
Copy link
Member

djowel commented Jul 1, 2020

If I may ask, can we enable Werror on demand only? this sometimes gives software packagers trouble, when they are using different compiler versions.

Yes, that is my intent. A CMake option perhaps?

/home/jpc/documents/projects/elements/lib/host/linux/window.cpp:52:23: error: lambda capture 'style_' is not used [-Werror,-Wunused-lambda-capture]
this, name, style_, bounds
~~^~~~~~
1 error generated.

And it is good that it detected that! Care for a PR? (Edit: of course you already did :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants