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

sccache's explicit language selection causes subtle behaviour changes #803

Closed
jdm opened this issue Jun 26, 2020 · 12 comments
Closed

sccache's explicit language selection causes subtle behaviour changes #803

jdm opened this issue Jun 26, 2020 · 12 comments

Comments

@jdm
Copy link

jdm commented Jun 26, 2020

Based on servo/mozangle#38, I tracked down the issue to the following:

  • we were using cc-rs to build a collection of C++ and C files into a static library
  • the C++ files required the -std=c++14 flag
  • clang via cc-rs had no problem building the C files with the -std=c++14 flag present
  • adding sccache to the build yielded an error that -std=c++14 couldn't be used with C files

A minimal testcase:

    cc::Build::new()
        .file("a.c")
        .cpp(true)
        .flag("-std=c++14")
        .compile("test");

When run with RUSTC_WRAPPER=sccache, this yields cargo:warning=error: invalid argument '-std=c++14' not allowed with 'C'. I can also reproduce without sccache by adding .flag("-xc"), which is what sccache does internally when invoking the compiler by guessing the language based on the filename.

@froydnj
Copy link
Contributor

froydnj commented Jun 26, 2020

@rickystewart , do you want to have a go at fixing this?

@rickystewart
Copy link
Contributor

Sure, but I won't be able to get to it awfully soon. Maybe in the next week. If it's more urgent than that, someone else should scoop it up.

@jdm
Copy link
Author

jdm commented Jun 26, 2020

One option might be to treat the presence of flags enabling C++ standards as C++ mode, regardless of the filename.

@glandium
Copy link
Collaborator

How does it even work without sccache?

$ gcc -std=c++14 test.c
cc1: warning: command line option ‘-std=c++14’ is valid for C++/ObjC++ but not for C
$ clang -std=c++14 test.c
error: invalid argument '-std=c++14' not allowed with 'C'

@glandium
Copy link
Collaborator

And in the GCC case, it doesn't enable C++ mode.

@glandium
Copy link
Collaborator

glandium commented Jun 26, 2020

Oh, here's why:

$ clang++ -std=c++14 test.c
clang-10: warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated [-Wdeprecated]
$ g++ -std=c++14 test.c

So to summarize:

Compiler Behavior
clang error
clang++ warning, compile as C++
gcc warning, compile as C
g++ compile as C++

Strictly speaking, sccache doesn't need to pass -x at all anymore... except in the dist case, where it needs to indicate the input comes from the preprocessor.

@glandium
Copy link
Collaborator

Note that clang++ warning means future clang versions could break you.

@rickystewart
Copy link
Contributor

So to be clear, is the preferred way to fix this to implement the behavior chart from glandium's comment above, rather than just blindly saying that we should treat the language as C++ if the std=c++XX tag is passed? I imagine that increased compatibility with the interfaces of the actual compilers is what we're trying to do.

@froydnj
Copy link
Contributor

froydnj commented Jul 7, 2020

So to be clear, is the preferred way to fix this to implement the behavior chart from glandium's comment above, rather than just blindly saying that we should treat the language as C++ if the std=c++XX tag is passed?

Yes, we should be deducing the behavior from the file extension, rather than the command-line arguments.

@rickystewart
Copy link
Contributor

That seems to contradict with the feature request and suggest that we should close this as WONTFIX, no? Deducing the behavior from the file extension is what we're doing now.

@jdm
Copy link
Author

jdm commented Jul 7, 2020

Is the solution to stop passing an explicit -x, per #803 (comment)?

@froydnj
Copy link
Contributor

froydnj commented Jul 7, 2020

That seems to contradict with the feature request and suggest that we should close this as WONTFIX, no? Deducing the behavior from the file extension is what we're doing now.

Sorry, I forgot the other piece: deducing the behavior from the compiler that's being invoked combined with the file extension.

Is the solution to stop passing an explicit -x, per #803 (comment)?

I think we could do that, but only in the local case, which means we still have to perform some kind of deduction in the dist case...and in some sense, as long as we're doing that, we might as well do it in both cases (?).

rickystewart pushed a commit to rickystewart/sccache that referenced this issue Jul 27, 2020
…Fixes mozilla#803

`g++` and `clang++` both compile `.c` files in C++ mode in the absence of any other directions (like the `-x` flag); add code to distinguish between the two compilers and their `++` variations, then fix up language selection and caching accordingly.
rickystewart pushed a commit to rickystewart/sccache that referenced this issue Jul 28, 2020
…Fixes mozilla#803

`g++` and `clang++` both compile `.c` files in C++ mode in the absence of any other directions (like the `-x` flag); add code to distinguish between the two compilers and their `++` variations, then fix up language selection and caching accordingly.
rickystewart pushed a commit to rickystewart/sccache that referenced this issue Aug 4, 2020
…Fixes mozilla#803

`g++` and `clang++` both compile `.c` files in C++ mode in the absence of any other directions (like the `-x` flag); add code to distinguish between the two compilers and their `++` variations, then fix up language selection and caching accordingly.
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

4 participants