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

Extend .ccls #171

Merged
merged 1 commit into from
Dec 22, 2018
Merged

Extend .ccls #171

merged 1 commit into from
Dec 22, 2018

Conversation

MaskRay
Copy link
Owner

@MaskRay MaskRay commented Dec 21, 2018

Fix #115 @madscientist

  • Add %h for C header files (the suffix .h is considered a C header, not a C++ header)
  • Add %hpp for C++ header files
  • If .ccls exists, it provides full command line for files not specified by compile_commands.json (before, compile_commands.json was ignored)
  • If the first line of .ccls is %compile_commands.json, it appends flags to compile_commands.json "arguments", instead of overriding.
    Files not specified by compile_commands.json will not be added to folder.entries, but their command line can be inferred from other files.

@madscientist
Copy link
Contributor

Wow thanks for the quick response! I'm testing now but something is not quite right. At first I was getting an error in Messages (maybe while ccls was rebuilding the index?) but now I get no errors but the header doesn't appear to be included. I do see the --include flag mentioned in the log though. I'll do some debugging provide more details.

@madscientist
Copy link
Contributor

I enabled lsp-print-io but it didn't seem to help me understand what's happening. Basically, I can see from the log that the %h option is being found; I see:

project.cc:247 I use /tmp/cclstest/.ccls: %h --include=global.h

but ccls still can't find these types.

I've created a test environment and attached it as a tar file here. Unpack it in /tmp (there are some hardcoded paths in compile_commands.json) then run emacs and visit the header file src/header.h and you'll see that size_t is not recognized as a valid type:

cd /tmp
tar xzf cclstest.tar.gz
emacs -nw -q -l ccls-test.el
C-x C-f cclstest/src/header.h

If I do the equivalent thing from the command line it works:

cd src
clang --include=global.h -I/tmp/cclstest/inc -o source.o -c source.c

even if I comment out the #include "global.h" from source.c, so this syntax does work properly from the command line.

cclstest.tar.gz

@madscientist
Copy link
Contributor

OK more details: it appears to be related to the combination of compile_commands.json and .ccls. If I remove compile_commands.json and change my .ccls to have all the flags:

%c
-I/tmp/cclstest/inc
%h
--include=global.h

then it works! If I put back compile_commands.json, even with the above .ccls, it doesn't work again.

BTW, I think a more formal definition of the syntax of .ccls in the wiki would be helpful; while the above works, if I put the option on the same line as %h

%h --include=global.h

then it doesn't work... is that intended? Using %c -I/tmp/cclstest/inc on one line does seem to work.

@MaskRay MaskRay force-pushed the project branch 2 times, most recently from 05f95d1 to 54e6f63 Compare December 22, 2018 01:26
@MaskRay
Copy link
Owner Author

MaskRay commented Dec 22, 2018

I pushed a commit to make -v=1 work: `ccls -log-file=/tmp/ls.log -v=1 "$@" will now log:

18:26:44 preamble     sema_manager.cc:706 I create session for /tmp/cclstest/src/source.c
18:26:44 preamble     sema_manager.cc:711 I  args: gcc -I/tmp/cclstest/inc -o source.o -c /tmp/cclstest/src/source.c --gcc-toolchain=/usr -working-directory=/tmp/cclstest/src

It should make it easy to observe what is going on.

I have fixed %h. It required some more work.

Note %c %h are modifiers that get stripped as the line prefix. You can't place the modified flag on the next line.

With a change to .ccls (it overrides compile_commands.json entries, instead of augmenting its command line with more flags), your example works for me:

% cat /tmp/cclstest/.ccls
%clang
%h --include=inc/global.h

@madscientist
Copy link
Contributor

Awesome, I'll give it a try.

I was confused about the precise syntax of the .ccls file; the https://github.com/MaskRay/ccls/wiki/Getting-started wiki page shows this syntax:

%clang
%c -std=gnu11
%cpp -std=gnu++14
%objective-c %objective-cpp -DGNUSTEP
%c %cpp -DFOO
-pthread

# Includes
-I/work/ccls/third_party
-I/work/ccls/another_third_party
# -I space_is_not_allowed

which implies that flags on a line by themselves are valid...? Maybe I misunderstood it.

@MaskRay MaskRay force-pushed the project branch 2 times, most recently from 9143ab3 to de26ec9 Compare December 22, 2018 04:37
@MaskRay MaskRay changed the title .ccls: add %h and %hpp; make .ccls augment compile_commands.json Extend .ccls Dec 22, 2018
@MaskRay MaskRay force-pushed the project branch 3 times, most recently from dbcfcd1 to c068f7b Compare December 22, 2018 05:51
@MaskRay
Copy link
Owner Author

MaskRay commented Dec 22, 2018

With your comment #115 (comment) in mind, I've changed the semantic a bit.

For you /tmp/cclstest example to work, you may also use:

% cat .ccls
%compile_commands.json
%h --include=global.h

See the updated description of the commit.

The relevant log when src/source.c and src/header.h (inferred from src/source.c) are opened:

22:32:54 preamble     sema_manager.cc:712 I create session for /tmp/cclstest/src/source.c
  gcc -I/tmp/cclstest/inc -o source.o -c /tmp/cclstest/src/source.c --gcc-toolchain=/usr -working-directory=/tmp/cclstest/src
22:33:42 preamble     sema_manager.cc:712 I create session for /tmp/cclstest/src/header.h
  gcc -I/tmp/cclstest/inc -o source.o -c /tmp/cclstest/src/header.h --include=global.h --gcc-toolchain=/usr -working-directory=/tmp/cclstest/src

I was confused about the precise syntax of the .ccls file

The source is still the best way to understand what's going on :) project.cc

      StringRef A(arg);
      if (A == "%clang") {
        args.push_back(lang == LanguageId::Cpp || lang == LanguageId::ObjCpp
                           ? "clang++"
                           : "clang");
      } else if (A[0] == '%') {
        bool ok = false;
        for (;;) {
          if (A.consume_front("%c "))
            ok |= lang == LanguageId::C;
          else if (A.consume_front("%h "))
            ok |= lang == LanguageId::C && header;
          else if (A.consume_front("%cpp "))
            ok |= lang == LanguageId::Cpp;
          else if (A.consume_front("%hpp "))
            ok |= lang == LanguageId::Cpp && header;
          else if (A.consume_front("%objective-c "))
            ok |= lang == LanguageId::ObjC;
          else if (A.consume_front("%objective-cpp "))
            ok |= lang == LanguageId::ObjCpp;
          else
            break;
        }
        if (ok)
          args.push_back(A.data());
      } else if (!excludeArgs.count(A)) {
        args.push_back(arg);
      }

@MaskRay
Copy link
Owner Author

MaskRay commented Dec 22, 2018

@Riatre @oblitum @bstaletic

@oblitum
Copy link

oblitum commented Dec 22, 2018

This didn't fix inclusion of headers at %cpp -I/opt/src/range-v3/include for me :-/ (#160).

@oblitum
Copy link

oblitum commented Dec 22, 2018

Can you give an example how %h and %hpp is supposed to be used? Is it for include directories or files?

@oblitum
Copy link

oblitum commented Dec 22, 2018

Can you give an example how %h and %hpp is supposed to be used? Is it for include directories or files?

Nevermind, I've noticed from discussion it's for files.

@madscientist
Copy link
Contributor

@MaskRay OK tested in my real code... it works great!! Now all my headers have full type information. Love it!

@oblitum Right, for files. It's for extra flags to be given to the compiler when ccls "compiles" header files alone, rather than as part of a source file. I don't expect it will be the most commonly-used option but it's critical in some situations.

@MaskRay MaskRay force-pushed the project branch 3 times, most recently from aa7eaf8 to 852cdef Compare December 22, 2018 18:30
@MaskRay MaskRay force-pushed the project branch 2 times, most recently from f7a7e43 to 7e696fa Compare December 22, 2018 18:34
@MaskRay
Copy link
Owner Author

MaskRay commented Dec 22, 2018

@oblitum For clang < 8, include_complete.cc (inherited from cquery) is used. I've commented at #160 (comment) The latest project branch should address #160

* Add %h for C header files (the suffix .h is considered a C header, not a C++ header)
* Add %hpp for C++ header files
* If .ccls exists, it provides full command line for files not specified by compile_commands.json (before, compile_commands.json was ignored)
* If the first line of .ccls is %compile_commands.json, it appends flags to compile_commands.json "arguments", instead of overriding.
  Files not specified by compile_commands.json will not be added to folder.entries, but their command line can be inferred from other files.

Also fix `#include <` completion of -I flags for clang < 8
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

Successfully merging this pull request may close these issues.

false diagnostics in include files due to never-firing #ifdefs
3 participants