Clang plugin: Handle CUDA files better
ClosedPublic

Authored by thomassc on Jan 1 2019, 11:26 PM.

Details

Summary

This fixes two issues in how KDevelop's clang plugin handles CUDA files (.cu source files and .cuh headers):

  1. Those file types were not treated as source files, therefore if such a file was modified, it was not passed among the list of modified source files to clang for re-parsing. The re-parsing thus incorrectly used the unmodified file on disk instead. This is addressed by the change to documentfinderhelpers.cpp and kdevclang.xml.
  2. The patch passes the general code parser settings also to CUDA files (change in parsesession.cpp). This is for example important to get flags such as -std=c++11 that are required for correct parsing. However, it is not correct: the build system may use separate options for CUDA files (e.g., CMake's CUDA_NVCC_FLAGS). But it might still be better than the previous behavior of not passing any options except -xcuda, since it might be somewhat likely that it is a reasonable guess.

Additional comments:
a) I am not sure whether the change in clanghelpers.cpp is required, but it seems coherent.
b) I changed the CUDA mime types in kdevclang.xml to inherit from text/x-c++src/hdr instead of text/x-csrc/hdr since the CUDA files can contain C++ code.
c) This patch is not sufficient to enable proper CUDA support. I'll post to the kdevelop-devel mailing list for that.

Test Plan

Tested manually.

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
thomassc created this revision.Jan 1 2019, 11:26 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptJan 1 2019, 11:26 PM
thomassc requested review of this revision.Jan 1 2019, 11:26 PM
apol added a subscriber: apol.Jan 2 2019, 5:51 PM

The patch looks good overall +1

plugins/clang/duchain/parsesession.cpp
108

Doesn't this need a space in the beginning?

thomassc added inline comments.Jan 2 2019, 11:00 PM
plugins/clang/duchain/parsesession.cpp
108

I don't think so, since the similar cases above and below this one also don't include a space as first character. Also, I verified with the clang executable that it accepts both "-x cuda" and "-xcuda".

kfunk accepted this revision.Jan 9 2019, 8:59 AM
kfunk added a subscriber: kfunk.

Looks good to me. Can you push to 5.3 branch yourself?

This revision is now accepted and ready to land.Jan 9 2019, 8:59 AM

I don't think that I have commit rights.

kfunk added a comment.Jan 9 2019, 12:51 PM

Could you give me your full name + email for attribution in the commit?

Ideally, you'd start using arcanist to properly upload diffs to Phabricator. They'll also contain the authoring details in that case.

Name: Thomas Schöps, Email: tom dot schoeps aet gmail dot com
Thanks for committing this. I'll try using arcanist next time.

This revision was automatically updated to reflect the committed changes.