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

Linter provides no warnings for invalid C code #828

Open
abbioro opened this issue Jun 14, 2017 · 18 comments
Open

Linter provides no warnings for invalid C code #828

abbioro opened this issue Jun 14, 2017 · 18 comments
Labels
Feature Request Language Service postponed In progress, but postponed in favor of other higher priority tasks
Milestone

Comments

@abbioro
Copy link

abbioro commented Jun 14, 2017

See title, create a fresh test.c file with the contents:

#include <stdio.h>

int main()
{
    asdfd;
    return 0;
}

(If) Linting is working properly, asdfd will be underlined with squigglies as you would expect. What you would not expect is that turning it into a function call makes the squigglies go away:

#include <stdio.h>

int main()
{
    asdfd();
    return 0;
}

Furthermore, hovering over it produces the popup int asdfd(), as if it was defined to return int somewhere. This is just silly.

Related to #746

@bobbrow
Copy link
Member

bobbrow commented Jun 15, 2017

Indeed. That is silly. We'll investigate.

@sean-mcmanus
Copy link
Collaborator

sean-mcmanus commented Jun 15, 2017

This only repros in C files, not C++. It looks like the IntelliSense compiler is using the old default int function return value C89 behavior, so asdfd() is being interpreted as a declaration for "int asdfd()", even though we're passing in the C11 flag.

@abbioro
Copy link
Author

abbioro commented Jun 15, 2017

Ok so it's valid C89, but surely this should be an error even in C89?

#include <stdio.h>
#include <stdlib.h>

int main()
{
    char *line = maloc(100);
    return 0;
}

The misspelled malloc line shows squigglies only when placed outside the main function, but no squigglies inside. Still shows the int hint.

@sean-mcmanus
Copy link
Collaborator

Visual Studio 2017 also shows maloc (or any other undefined function call identifier) as being a function returning an int. I'll ask the VS-side people if they think our shared compiler has a bug.

@sean-mcmanus
Copy link
Collaborator

While it's not valid in the C99 standard, it looks like all the C compilers we support don't support this rule, I assume because it would be a breaking change with C89. So this is currently "by design", but if you can locate us a compiler (or linter) that enforces the non-implicit int behavior, we theoretically could add a new flag to trigger the enforcement behavior.

@abbioro
Copy link
Author

abbioro commented Jun 15, 2017

Sorry I'm not sure I understand, the last example I posted doesn't compile with either GCC or Clang even with -std=c89, yet VS Code doesn't show any error squigglies for it. Is it not an error? Something is definitely wrong here. There should at least be warnings.

test.c:8:11: warning: incompatible integer to pointer conversion initializing 'char *' with an expression of type 'int' [-Wint-conversion]
    char *line = maloc(100);
          ^      ~~~~~~~~~~
1 warning generated.
/tmp/test-3f53c3.o: In function `main':
test.c:(.text+0x17): undefined reference to `maloc'
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@sean-mcmanus
Copy link
Collaborator

Thanks for the info. The compiler is not giving you an error -- it successfully compiles and fails during linking. IntelliSense currently only gives compiler errors. This sounds like a feature request for warnings and/or linker errors.

@abbioro
Copy link
Author

abbioro commented Jun 15, 2017

Warnings + linker errors sounds good to me.

@bobbrow bobbrow changed the title Function names are never undefined in linter Linter provides no warnings for invalid C code Jun 16, 2017
@sean-mcmanus
Copy link
Collaborator

FYI, you can enable the warnings "experimental/hidden" feature via modifiying the msvc.json file next to the binary by removing the --no_warnings flag. Not sure when we'll get around to adding a setting for that. VS 2017 doesn't have these warnings enabled so it hasn't been tested as much. A previous issue reported was #2091 .

image

@sean-mcmanus sean-mcmanus added the help wanted Can be fixed in the public (open source) repo. label Aug 2, 2018
@sean-mcmanus sean-mcmanus added this to the August 2018 milestone Aug 8, 2018
@sean-mcmanus sean-mcmanus self-assigned this Aug 8, 2018
@sean-mcmanus sean-mcmanus removed the help wanted Can be fixed in the public (open source) repo. label Aug 8, 2018
@sean-mcmanus sean-mcmanus removed their assignment Aug 9, 2018
@sean-mcmanus sean-mcmanus added the fixed Check the Milestone for the release in which the fix is or will be available. label Aug 9, 2018
This was referenced Apr 2, 2019
@github-actions
Copy link

This feature request has received enough votes to be added to our backlog.

@alexanderkutin
Copy link

FYI, you can enable the warnings "experimental/hidden" feature via modifiying the msvc.json file next to the binary by removing the --no_warnings flag. Not sure when we'll get around to adding a setting for that. VS 2017 doesn't have these warnings enabled so it hasn't been tested as much. A previous issue reported was #2091 .

image

Is it possible to enable "experimental/hidden" feature in Linux?

@sean-mcmanus
Copy link
Collaborator

@alexanderkutin Yes, the previous comment I made is outdated -- the file to modify is now common.json (not msvc.json).

@esotericist
Copy link

it seems that compilers generally support -Werror-implicit-function-declaration to produce errors instead of warnings for implicit functions. is there a way to get this parameter passed to the compiler for the intellisense behavior?

@sean-mcmanus
Copy link
Collaborator

@esotericist The warnings haven't been added to IntelliSense, but they'll be available via clang-tidy, but we're still working on the feature, so we don't know yet when it'll be available (see #2908):
image

@esotericist
Copy link

i think my question was misunderstood. i'm not asking about warning support in vs code.

it is possible to tell the compiler to treat them as errors, not warnings, without treating all warnings as errors.

i can do this with my build setup, but i was wanting to know if we could pass that argument through vs code so they're treated as errors (not warnings) in the UI.

@sean-mcmanus
Copy link
Collaborator

sean-mcmanus commented Oct 5, 2021

@esotericist The -Werror-implicit-function-declaration is a clang or gcc flag, which our IntelliSense compiler doesn't process. You'd need to use the flag with your compiler or clang-tidy, and then it'll switch it from a warning to an error:
image

i.e. our IntelliSense process still considers it a "warning" regardless of whether flags are added to treat warnings as errors.

@Heath123
Copy link

Heath123 commented Jul 4, 2022

FYI, you can enable the warnings "experimental/hidden" feature via modifiying the msvc.json file next to the binary by removing the --no_warnings flag. Not sure when we'll get around to adding a setting for that. VS 2017 doesn't have these warnings enabled so it hasn't been tested as much. A previous issue reported was #2091 .

image

What about if I'm using GCC on Linux instead of MSVC?

@sean-mcmanus
Copy link
Collaborator

Linter errors for invalid C code can be shown now via clang-tidy (Right-click "Run Code Analysis on Active File", C_Cpp.codeAnalysis.clangTidy.enabled, etc.):

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Language Service postponed In progress, but postponed in favor of other higher priority tasks
Projects
None yet
Development

No branches or pull requests

6 participants