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

Support partial preprocessing with clang #37

Open
jpeach opened this issue Jun 5, 2021 · 3 comments
Open

Support partial preprocessing with clang #37

jpeach opened this issue Jun 5, 2021 · 3 comments

Comments

@jpeach
Copy link
Contributor

jpeach commented Jun 5, 2021

This one made me scratch my head a bit because I assumed it was a Bazel problem and didn't look in the llama docs for it 😂

ERROR: /home/jpeach/.cache/bazel/_bazel_jpeach/bca846880028b2eb76664b66797b90da/external/com_google_protobuf/BUILD:129:11: Compiling src/google/protobuf/stubs/stringprintf.cc failed: (Exit 1): llamacc failed: error executing command 
  (cd /home/jpeach/.cache/bazel/_bazel_jpeach/bca846880028b2eb76664b66797b90da/sandbox/linux-sandbox/12/execroot/build && \
  exec env - \
    PATH=/bin:/usr/bin:/usr/local/bin \
    PWD=/proc/self/cwd \
  /home/jpeach/src/envoy-build/bazel/llamacc -U_FORTIFY_SOURCE -fstack-protector -Wall -fno-omit-frame-pointer -g0 -O2 '-D_FORTIFY_SOURCE=1' -DNDEBUG -ffunction-sections -fdata-sections '-std=c++0x' -MD -MF bazel-out/k8-opt-exec-2B5CBBC6/bin/external/com_google_protobuf/_objs/protobuf_lite/stringprintf.d '-frandom-seed=bazel-out/k8-opt-exec-2B5CBBC6/bin/external/com_google_protobuf/_objs/protobuf_lite/stringprintf.o' -iquote external/com_google_protobuf -iquote bazel-out/k8-opt-exec-2B5CBBC6/bin/external/com_google_protobuf -isystem external/com_google_protobuf/src -isystem bazel-out/k8-opt-exec-2B5CBBC6/bin/external/com_google_protobuf/src -g0 -g0 -DHAVE_PTHREAD -DHAVE_ZLIB -Woverloaded-virtual -Wno-sign-compare -Wno-unused-function -Wno-write-strings -Wno-deprecated-declarations -Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"' '-D__TIME__="redacted"' -c external/com_google_protobuf/src/google/protobuf/stubs/stringprintf.cc -o bazel-out/k8-opt-exec-2B5CBBC6/bin/external/com_google_protobuf/_objs/protobuf_lite/stringprintf.o)
Execution platform: @local_config_platform//:host

Use --sandbox_debug to see verbose messages from the sandbox
clang: error: unknown argument: '-fdirectives-only'
Target @envoy//source/exe:envoy-static failed to build

-fdirectives-only doesn't work with clang. Perhaps there's an alternative that works with both compilers? There is a short thread discussing how clang's -frewrite-includes differs, but maybe the difference doesn't matter for llama?

@nelhage
Copy link
Owner

nelhage commented Jun 6, 2021

Yeah, I think -frewrite-includes would work for us, but I'm not sure if there's an easy way to autodetect which one to use. Could add another configuration flag, I guess -- LLAMACC_CC_IS_CLANG or something -- but that seems mildly unpleasant

@jpeach
Copy link
Contributor Author

jpeach commented Jun 12, 2021

FWIW, I tried this quick hack with the Envoy build, and it ended up not being able to resolve libstdc++ types properly. Not sure whether there's something more needed to get partial pre-processing to work with clang, or whether this Bael making some aggressive assumptions.

--- cmd/llamacc/main.go
+++ cmd/llamacc/main.go
@@ -196,7 +196,7 @@ func buildLocalPreprocess(ctx context.Context, client *daemon.Client, cfg *Confi
                preprocessor.Args = []string{comp.LocalCompiler(cfg)}
                preprocessor.Args = append(preprocessor.Args, comp.LocalArgs...)
                if !cfg.FullPreprocess {
-                       preprocessor.Args = append(preprocessor.Args, "-fdirectives-only")
+                       preprocessor.Args = append(preprocessor.Args, "-frewrite-includes")
                }
                preprocessor.Args = append(preprocessor.Args, "-E", "-o", "-", comp.Input)
                preprocessor.Stdout = &preprocessed
@@ -224,7 +224,7 @@ func buildLocalPreprocess(ctx context.Context, client *daemon.Client, cfg *Confi
        args.Args = []string{comp.RemoteCompiler(cfg)}
        args.Args = append(args.Args, comp.RemoteArgs...)
        if !cfg.FullPreprocess {
-               args.Args = append(args.Args, "-fdirectives-only", "-fpreprocessed")
+               args.Args = append(args.Args, "-frewrite-includes")
        }
        args.Args = append(args.Args, "-x", comp.PreprocessedLanguage, "-o", comp.Output, "-")

@jpeach
Copy link
Contributor Author

jpeach commented Jun 12, 2021

OK, I figured this one out. We need to pass all the Defs along to the remote build as well as the -frewrite-includes flag. Clang also needs different constants in preprocessedLang (ie. "c" and "c++").

jpeach added a commit to jpeach/llama that referenced this issue Jun 13, 2021
When doing partial local preprocessing, a remote clang compilation
still needs the preprocessor definitions from the command line so
that the remaining preprocessing can be performed correctly. Partial
preprocessing is necessary to prevent the remote compiler issuing
spurious warnings from macro expansion that would be suppressed in
a local compilation.

This means when spawning a compilation, we should always use either
`LocalArgs` or `RemoteArgs`, and the need to separately preserve
unknown arguments and conditionally restore preprocessor definitions
goes away.

This updates nelhage#37.
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

2 participants