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

ccache gives different warnings than raw g++ call #93

Closed
tsdgeos opened this issue Jun 7, 2016 · 13 comments
Closed

ccache gives different warnings than raw g++ call #93

tsdgeos opened this issue Jun 7, 2016 · 13 comments
Labels
new compiler behavior New compiler behavior not handled correctly
Milestone

Comments

@tsdgeos
Copy link

tsdgeos commented Jun 7, 2016

Output from g++ and ccache: http://paste.ubuntu.com/17091648/

The file i'm compiling is at http://paste.ubuntu.com/17091697/

$ ccache -V
ccache version 3.2.4

$ g++ --version
g++ (Ubuntu 5.3.1-14ubuntu2.1) 5.3.1 20160413

Using Ubuntu 16.04

Any idea why that happens?

@jrosdahl
Copy link
Member

Can't reproduce the problem using ccache 3.2.5 and g++ 5.2.1 to compile your example code.

It might be useful to have a look at ccache's log output:

CCACHE_LOGFILE=/dev/stdout ccache g++ -c q.cpp -isystem /usr/include/x86_64-linux-gnu/qt5/ -fPIC -Wsuggest-override

@tsdgeos
Copy link
Author

tsdgeos commented Jun 13, 2016

Interesting, it's the -E step that adds the output, i.e.
$ g++ -c -fPIC -Wsuggest-override -isystem /usr/include/x86_64-linux-gnu/qt5/ q.cpp
$ g++ -fPIC -Wsuggest-override -isystem /usr/include/x86_64-linux-gnu/qt5/ -E q.cpp > preprocess.cpp
$ g++ -fPIC -Wsuggest-override -isystem /usr/include/x86_64-linux-gnu/qt5/ -c preprocess.cpp
q.cpp:5:75: warning: ‘virtual const QMetaObject* Moo::metaObject() const’ can be marked override [-Wsuggest-override]
q.cpp:5:109: warning: ‘virtual void* Moo::qt_metacast(const char*)’ can be marked override [-Wsuggest-override]
q.cpp:5:148: warning: ‘virtual int Moo::qt_metacall(QMetaObject::Call, int, void**)’ can be marked override [-Wsuggest-override]

I guess that makes it a gcc bug?

@tsdgeos
Copy link
Author

tsdgeos commented Jun 13, 2016

Not sure i understand the g++ devs correctly but i think they say it's not really a bug but a byproduct of doing the -E step

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71517

Is the -E step necessary for ccache to work?

And would switching to using something like

$ g++ -E -fdirectives-only -fPIC -Wsuggest-override -isystem /usr/include/x86_64-linux-gnu/qt5/ q.cpp > q.i
$ g++ -c -fpreprocessed -fdirectives-only -fPIC -Wsuggest-override -isystem /usr/include/x86_64-linux-gnu/qt5/ q.i

Would be possible? (since that also seems to make the warnings go away)

@jakubjelinek
Copy link

You don't want -fdirectives-only on the -c compilation, only on the preprocessing step obviously.

@afbjorklund
Copy link
Contributor

afbjorklund commented Jun 13, 2016

If you don't give -fdirectives-only to both -E and -c, you will get compilation errors ?
This is because macros will be left unexpanded, and choked upon by -fpreprocessed

You can use run_second_cpp / $CCACHE_CPP2, to run the preprocessor (cpp) again.
That is, pass the .cc file to the compiler (c++) instead of using the (preprocessed) .ii file.

@jakubjelinek
Copy link

The second step should be of course without -fpreprocessed too, because what -E -fdirectives-only does is not preprocessing, it is just evaluating the various preprocessing conditions and including the headers.

@jrosdahl
Copy link
Member

jrosdahl commented Jul 8, 2016

You don't want -fdirectives-only on the -c compilation, only on the preprocessing step obviously.

I understand why you suggest using -fdirectives-only, but doing so would introduce at least two problems for ccache:

  1. ccache will no longer work for compilers that lack -fdirectives-only, for instance older GCC versions.
  2. It will no longer be possible to get ccache hits for irrelevant -D command-line options.

@jakubjelinek
Copy link

On Fri, Jul 08, 2016 at 08:46:26AM -0700, Joel Rosdahl wrote:

You don't want -fdirectives-only on the -c compilation, only on the preprocessing step obviously.

I understand why you suggest using -fdirectives-only, but doing so would introduce at least two problems for ccache:

  1. ccache will no longer work for compilers that lack -fdirectives-only, for instance older GCC versions.

That can surely be configurable and/or runtime testable, so one could
configure ccache that only supports only -fdirectives-only, only -E, or
both and decide at runtime if the compiler supports -fdirectives-only or
not.

  1. It will no longer be possible to get ccache hits for irrelevant -D command-line options.

The amount of diagnostics etc. that differs between non-integrated and
integrated preprocessing significantly increases every release, so 2.
is small enough price to pay for that. E.g. in GCC 6 the
-Wmisleading-indentation warning, which is included in -Wall, or decisions
whether some warning should be not emitted because of its origin from system
headers etc. are something where the normal preprocessing loses so much
information that it can't behave reliably afterwards.

Jakub

@jrosdahl
Copy link
Member

That can surely be configurable

Possible: yes. Unproblematic: no.

Requiring configuration to make ccache work with certain compilers would be quite painful from a ccache user point of view since ccache always has tried to work out of the box regardless of which compiler is being used. Also, there's (at least currently) no good way to have compiler-specific configuration since ccache doesn't really know which compiler is run. So to preserve the works-out-of-the-box and no-compiler-specific-configuration traits, I think that ccache would need runtime detection. Which leads to:

and/or runtime testable

If you mean testing for compiler version (with something like "$compiler --version", or if that fails, "$compiler -V" for e.g. SUNStudio, or if that fails, "$compiler /HELP" for MSVC, or if that fails, what?) for each ccache invocation, then: Possible: yes. Easy to implement: probably. Easy to maintain: probably not. Will slow down ccache, especially for esoteric compilers: yes.

(By the way, one might think that it should be possible to cache the compiler detection result based on mtime of the compiler, and that's of course possible but not very robust since the compiler could be a shell script that selects the actual compiler to run.)

I have considered introducing runtime detection several times before but never got to try implementing it for the reasons mentioned above.

If you mean testing specifically if -fdirectives-only works, then that has similar problems.

The amount of diagnostics etc. that differs between non-integrated and integrated preprocessing significantly increases every release, so 2. is small enough price to pay for that.

It may be a small enough price for you who probably doesn't even use ccache, but for me and likely other ccache users, getting cache misses just because some unused -D option changed value would matter. That said, I'm of course in no position to do much else than try to compensate for changed behavior of compilers.

Since the main compilers used with ccache are GCC and clang, and, as you say, both likely will continue to work differently when compiling preprocessed input vs normal compilation, it makes sense to follow their new behavior. However, since clang doesn't have -fdirectives-only, it wouldn't help the clang case even if ccache started using it.

Therefore, my suggestion is to flip the default of run_second_cpp from false to true. This will solve the problem for all compilers at the expense of some slowdown. For modern compilers the slowdown is measurable but quite tiny, so I don't think it's a problem. To me, this would be much more appealing than e.g. losing the "ignore unused -D flags" feature.

@itensionanders and others: Opinions on simply letting run_second_cpp default to true?

@afbjorklund
Copy link
Contributor

@jrosdahl : I think that we should consider enabling both of run_second_cpp and hash_dir

The usual case is direct hit anyway, so enabling it wouldn't cost that much - unless compiling.
And it would give much better support for GCC 5+ and Clang out-of-the-box, with an opt-out ?

The hash_dir similarly gives less surprises when compiling with debug objects, and now there
is proper support for -fdebug-prefix-map to avoid it. And again there is opt-out, there too.

I've found that -fdirectives-only is not enough anyway, to avoid cases like the one in #105

@afbjorklund
Copy link
Contributor

This and other similar whitespace diagnostics probably also has problems using distcc, since that also transports preprocessed source code (using the ccache intermediate files, where available).

@jrosdahl
Copy link
Member

Yes, agree.

@jrosdahl
Copy link
Member

#116 and #117 track the progress of flipping defaults of run_second_cpp and hash_dir. Closing this issue.

@jrosdahl jrosdahl added bug Does not work as intended/documented new compiler behavior New compiler behavior not handled correctly and removed support bug Does not work as intended/documented labels Dec 27, 2020
@jrosdahl jrosdahl added this to the 3.3 milestone Dec 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new compiler behavior New compiler behavior not handled correctly
Projects
None yet
Development

No branches or pull requests

4 participants