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

Flychecker for CUDA using nvcc #1508

Closed
wants to merge 1 commit into from

Conversation

@KentJames
Copy link
Contributor

commented Sep 29, 2018

This is a basic flychecker for the NVIDIA cuda language. This should work within the standard cuda-mode defined here: https://www.emacswiki.org/emacs/CudaMode

I have my own custom cuda-mode which is an extension of the one above, which I'll release shortly, but the naming is the same so shouldn't affect flycheck.

nvcc abstracts the underlying system compiler that it uses, so the errors reported by nvcc and picked up by flycheck should be consistent.

@fmdkdd
Copy link
Member

left a comment

Sorry for the delay, and thank you for the contribution! Always nice to see checkers for languages we did not already cover.

I left some comments, please address them. To pass the CI, you'll need to document your checker and its options under doc/languages.rst. You'll also need to fix the formatting error in flycheck.el. You can use the make format target to do it for you (see the Contributors Guide).

Additionally, it would be best if we could add nvcc to the Docker image used to run the integration tests, but it looks like the CUDA toolkit is rather large, so maybe we can hold on that. Please just make sure that make LANGUAGE=cuda integ passes your tests.

flycheck.el Outdated Show resolved Hide resolved
flycheck.el Outdated Show resolved Hide resolved
flycheck.el Outdated Show resolved Hide resolved
flycheck.el Show resolved Hide resolved
@KentJames

This comment has been minimized.

Copy link
Contributor Author

commented Oct 19, 2018

@CLAassistant

This comment has been minimized.

Copy link

commented Oct 21, 2018

CLA assistant check
All committers have signed the CLA.

@KentJames

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2018

I'm struggling to get this to pass. I'm unsure as to why it's complaining about not-documented options when they are documented in languages.rst.

It's almost like the checker isn't registered, but it is very explicitly registered. Any ideas?

@KentJames

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2018

Okay looks like it's all there now. Tests pass, and travis is happy.

@fmdkdd
Copy link
Member

left a comment

Thanks! Just a few minor changes and this should be good.

Please also add a line to CHANGES.rst announcing your new checker!

And lastly, please squash your commits as one before we can merge.

flycheck.el Outdated Show resolved Hide resolved
flycheck.el Outdated Show resolved Hide resolved
doc/languages.rst Outdated Show resolved Hide resolved
doc/languages.rst Outdated Show resolved Hide resolved
flycheck.el Outdated Show resolved Hide resolved
flycheck.el Outdated Show resolved Hide resolved
flycheck.el Outdated Show resolved Hide resolved
flycheck.el Outdated Show resolved Hide resolved

@KentJames KentJames force-pushed the KentJames:flycheck-cuda branch from 0606cf1 to d585b37 Oct 28, 2018

@KentJames

This comment has been minimized.

Copy link
Contributor Author

commented Oct 28, 2018

I'm a bit mystified why Travis didn't pass that? The lint-python test threw up loads of errors in files I haven't touched. But it passed fine last time... makes no sense.

@fmdkdd

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

The lint-python test threw up loads of errors in files I haven't touched. But it passed fine last time... makes no sense.

Yes, I had the same issue in #1513. I don't know why this acted all of a sudden. flake8 should not upgrade itself since we are using a cached version. Anyway, I've extracted the fix from #1513 and pushed to master. You should be able to rebase to grab the fix.

@fmdkdd

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

Also, please use Flycheck for CUDO using nvcc as the first line of your squashed commit; that will be more descriptive.

@fmdkdd

fmdkdd approved these changes Oct 29, 2018

Flychecker for CUDA using nvcc
Fix link

Add tests

Tidy up to match flycheck conventions for PR

Fix formatting

Turns out you need that indent

Tidy up to align with flycheck conventions

@KentJames KentJames force-pushed the KentJames:flycheck-cuda branch from d585b37 to b708b0c Oct 29, 2018

@fmdkdd

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

Rebased and merged in #1554. Thanks @KentJames!

@fmdkdd fmdkdd closed this Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.