search

Sarcasm / flycheck-irony

C, C++ and Objective-C support for Flycheck, using Irony Mode
56 stars 10 forks source link

Ignore useless compiler warning messages #10

Closed hotpxl closed 9 years ago

hotpxl commented 9 years ago

YouCompleteMe has this piece of comment and code. I think it's quite useful to port it here. I added a variable flycheck-irony--ignored-error so we could ignore errors before passing it on to flycheck.

Clang has an annoying warning that shows up when we try to compile header files if the header has "#pragma once" inside it. The error is not legitimate because it shows up because libclang thinks we are compiling a source file instead of a header file.

See our issue #216 and upstream bug: http://llvm.org/bugs/show_bug.cgi?id=16686

The second thing we want to filter out are those incredibly annoying "too many errors emitted" diagnostics that are utterly useless.

hotpxl commented 9 years ago

Oh. I didn't filter the "too many errors emitted" thing. Partly because I haven't encountered one yet (yes, I rarely make errors :100: ) and I don't want to be too radical.

Sarcasm commented 9 years ago

I have the feeling that we are fixing the wrong problem here. Did you look at the Clang code to know whether or not you could fix it upstream?

How does it work with translated diagnostics? I guess if irony support another syntax checker than flycheck we might want this feature too, so should probably be in irony-diagnostics.el.

hotpxl commented 9 years ago

I have the feeling that we are fixing the wrong problem here. Did you look at the Clang code to know whether or not you could fix it upstream?

It could be an upstream bug. It could be an abuse of libclang, depending on how you view this problem. But anyway it needs fixing and I guess this could be the easiest way. Also we definitely need a way to filter the result.

How does it work with translated diagnostics?

I don't have a translated version now. But since the filter is not aggressive. It only filters out things that fully match. Then we could add more if there is this need. Or the users could setq for themselves.

I guess if irony support another syntax checker than flycheck we might want this feature too, so should probably be in irony-diagnostics.el.

I am not sure. I think this is kind of frontend filtering.

Sarcasm commented 9 years ago

It could be an upstream bug. It could be an abuse of libclang, depending on how you view this problem.

It is certainly an upstream bug (http://llvm.org/bugs/show_bug.cgi?id=16686). On irony-mode side, what is the "bug" is that we do not tell Clang that the file is a "header".

But anyway it needs fixing and I guess this could be the easiest way.

Not for me, it adds maintenance while this could be fixed once and for all in Clang.

Also we definitely need a way to filter the result.

Sure, but I doubt I would filter only a fixed string like this. And maybe, the one that should be able to filter is flycheck directly, not irony (maybe not).

I am not sure. I think this is kind of frontend filtering.

I'm not sure, it's a Clang bug, so it is far from a frontend issue. This warning shouldn't even be created.

FYI, before I asked you, I looked at the Clang code (https://github.com/llvm-mirror/clang/blob/f1079892295172a63033442f345dbfbf12011579/lib/Lex/Pragma.cpp#L353-L356), and I'm sure someone with little knowledge could be able to come up with something. Participating to Clang development is always an enlightening experience.

Sarcasm commented 9 years ago

Note: If some attempt to fix Clang is made, I would still be glad to do a version check on clang and hide this warning on older version.

hotpxl commented 9 years ago

Just put aside the "pragma" problem for now.

Probably I want a general "filter". I have other things to filter. Then could you add a variable that's says "filter". It is by default nil. But the user could provide a lambda to filter out things the user doesn't want to see.

Not for me, it adds maintenance while this could be fixed once and for all in Clang.

This won't add maintenance. Because it's by default nil. And if the user is able to write some lambda, at least he has some knowledge to it. It's user's responsibility to get the filter right.

Sure, but I doubt I would filter only a fixed string like this.

Well, you could do anything with it. Not a fixed string anymore, and you are not supposed to be aware of it. If it breaks anything, again, it's the user's responsibility to make it right.

hotpxl commented 9 years ago

Or, I see that flycheck provided a error-filter here. It's set to identity now. I guess we could expose it as a variable that's identity by default?

Sarcasm commented 9 years ago

I guess that is an acceptable thing to do, to expose :error-filter.

Sarcasm commented 9 years ago

Now that #11 is merged, I'm not against creating a filter variable to filter this error, even though I don't think that is the best way to tackle this issue.

hotpxl commented 9 years ago

We could provide a util function that filters error according to a list. The user could opt not to use this one of course. How about this?

hotpxl commented 9 years ago

Probably a helper function is too adhoc to be bundled. But I guess regular emacs users could do this.