[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

[Tao Liang via Phabricator via cfe-commits]

Origami404 created this revision.
Herald added a project: All.
Origami404 added reviewers: aaron.ballman, erichkeane, mgorny, jyknight, 
mibintc, clang-language-wg.
Origami404 published this revision for review.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

On GNU/Linux, GCC will automatically include `stdc-predefs.h`, while clang does
not. That is OK for glibc system because glibc includes this file manually. But
with other libc (e.g. musl), which does not manually include this, clang will
fail but GCC can get it compiled.

In 2017, D34158  try to introduce a new flag 
called `fsystem-include-if-exists`
to fix this, but it was reverted. Nearly, D106577 
 points out that macro
`__STDC_ISO_10646__` was indeed needed, which is defined in `stdc-predefs.h`.
After a discussion, the community reaches a consensus, to make this macro
available with a D34158 -like method.

In this patch, we port the patch in D34158  
into the current clang codebase. The
change is almost the same with D34158 , but we 
change some tests to suit current
codes:

1. c-index-test now does not accept a `-ffreestanding` flag, and it should not.

We choose to disable this pre-include action in Objective-C because it will
cause some c-index-test's tests on Objective-C to fail.

2. Some unit tests about parsing and re-parsing will be affected by this change.

To keep such tests passed, we add `-ffreestanding` flags to them.

3. The new tool, Clang-Scan-Deps, will analyze all header files to find

dependencies. After we add an implicit include, its result will have an extra
`stdc-predefs.h`. Currently, we choose to add `-ffreestanding` flags to all the
affected tests for it. But maybe making it ignore the default header will be a
better choice.

Signed-off-by: Tao Liang 
Co-authored-by: YingChi Long 

Link: https://reviews.llvm.org/D34158
Link: https://gcc.gnu.org/gcc-4.8/porting_to.html
Link: https://reviews.llvm.org/D106577


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137043

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/Driver/Job.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/Linux.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/ClangScanDeps/Inputs/has_include_if_elif.json
  clang/test/ClangScanDeps/Inputs/header_stat_before_open_cdb.json
  clang/test/ClangScanDeps/Inputs/headerwithdirname.json
  clang/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
  clang/test/ClangScanDeps/Inputs/module_fmodule_name_cdb.json
  clang/test/ClangScanDeps/Inputs/modules-context-hash/cdb_a.json.template
  clang/test/ClangScanDeps/Inputs/modules-context-hash/cdb_b.json.template
  clang/test/ClangScanDeps/Inputs/modules-pch-common-submodule/cdb_pch.json
  clang/test/ClangScanDeps/Inputs/modules-pch-common-submodule/cdb_tu.json
  clang/test/ClangScanDeps/Inputs/modules-pch-common-via-submodule/cdb_pch.json
  clang/test/ClangScanDeps/Inputs/modules-pch-common-via-submodule/cdb_tu.json
  clang/test/ClangScanDeps/Inputs/modules-pch/cdb_pch.json
  clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu.json
  clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu_with_common.json
  clang/test/ClangScanDeps/Inputs/modules_cdb.json
  clang/test/ClangScanDeps/Inputs/modules_inferred_cdb.json
  clang/test/ClangScanDeps/Inputs/no-werror.json
  clang/test/ClangScanDeps/Inputs/preprocess_minimized_pragmas_cdb.json
  clang/test/ClangScanDeps/Inputs/regular_cdb.json
  clang/test/ClangScanDeps/Inputs/relative_directory.json
  clang/test/ClangScanDeps/Inputs/static-analyzer-cdb.json
  clang/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json
  clang/test/ClangScanDeps/Inputs/symlink_cdb.json
  clang/test/ClangScanDeps/Inputs/target-filename-cdb.json
  clang/test/ClangScanDeps/Inputs/vfsoverlay_cdb.json
  clang/test/ClangScanDeps/header-search-pruning-transitive.c
  clang/test/ClangScanDeps/modules-dep-args.c
  clang/test/ClangScanDeps/modules-header-sharing.m
  clang/test/ClangScanDeps/modules-implementation-module-map.c
  clang/test/ClangScanDeps/modules-implicit-dot-private.m
  clang/test/ClangScanDeps/modules-incomplete-umbrella.c
  clang/test/ClangScanDeps/modules-no-undeclared-includes.c
  clang/test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
  clang/test/Driver/stdc-predef.c
  clang/test/Driver/stdc-predef.i
  clang/unittests/Tooling/TestVisitor.h
  clang/unittests/libclang/LibclangTest.cpp

Index: clang/unittests/libclang/LibclangTest.cpp
===
--- clang/unittests/libclang/LibclangTest.cpp
+++ clang/unittests/libclang/LibclangTest.cpp
@@ -366,8 +366,11 @@
 "printf(\"mmm!!\");\n"
 "#endif");
 
-  ClangTU = clang

[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

[Tao Liang via Phabricator via cfe-commits]

Origami404 updated this revision to Diff 471865.
Origami404 added a comment.

I'm sorry that my patch can not pass the CI test, but I can not reproduce the CI
failure loaclly, including clang-format related problems. On my machine, 
`ninja check-clang` shows:

  [0/1] Running the Clang regression tests
  llvm-lit: 
/home/origami/llvm/llvm-project/llvm/utils/lit/lit/llvm/config.py:456: note: 
using clang: /home/origami/llvm/llvm-project/build-release/bin/clang
  
  Testing Time: 183.65s
Skipped  :35
Unsupported  :  1681
Passed   : 29839
Expectedly Failed:26

And `git clang-format HEAD~1` show:

  clang-format did not modify any files

I am on Fedora 36, with clang-format 14.0.5 and python 3.10.7. I guess the test
failure maybe lead to incorrect platform setting, so I change the new regression
test in this patch to only run on Linux now by adding `-target` triple.

I am not familiar with the LLVM pybindings, but I will be more than appreciative
if someone can tell me how to fix the CI failure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137043/new/

https://reviews.llvm.org/D137043

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/Driver/Job.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/Linux.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/ClangScanDeps/Inputs/has_include_if_elif.json
  clang/test/ClangScanDeps/Inputs/header_stat_before_open_cdb.json
  clang/test/ClangScanDeps/Inputs/headerwithdirname.json
  clang/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
  clang/test/ClangScanDeps/Inputs/module_fmodule_name_cdb.json
  clang/test/ClangScanDeps/Inputs/modules-context-hash/cdb_a.json.template
  clang/test/ClangScanDeps/Inputs/modules-context-hash/cdb_b.json.template
  clang/test/ClangScanDeps/Inputs/modules-pch-common-submodule/cdb_pch.json
  clang/test/ClangScanDeps/Inputs/modules-pch-common-submodule/cdb_tu.json
  clang/test/ClangScanDeps/Inputs/modules-pch-common-via-submodule/cdb_pch.json
  clang/test/ClangScanDeps/Inputs/modules-pch-common-via-submodule/cdb_tu.json
  clang/test/ClangScanDeps/Inputs/modules-pch/cdb_pch.json
  clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu.json
  clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu_with_common.json
  clang/test/ClangScanDeps/Inputs/modules_cdb.json
  clang/test/ClangScanDeps/Inputs/modules_inferred_cdb.json
  clang/test/ClangScanDeps/Inputs/no-werror.json
  clang/test/ClangScanDeps/Inputs/preprocess_minimized_pragmas_cdb.json
  clang/test/ClangScanDeps/Inputs/regular_cdb.json
  clang/test/ClangScanDeps/Inputs/relative_directory.json
  clang/test/ClangScanDeps/Inputs/static-analyzer-cdb.json
  clang/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json
  clang/test/ClangScanDeps/Inputs/symlink_cdb.json
  clang/test/ClangScanDeps/Inputs/target-filename-cdb.json
  clang/test/ClangScanDeps/Inputs/vfsoverlay_cdb.json
  clang/test/ClangScanDeps/header-search-pruning-transitive.c
  clang/test/ClangScanDeps/modules-dep-args.c
  clang/test/ClangScanDeps/modules-header-sharing.m
  clang/test/ClangScanDeps/modules-implementation-module-map.c
  clang/test/ClangScanDeps/modules-implicit-dot-private.m
  clang/test/ClangScanDeps/modules-incomplete-umbrella.c
  clang/test/ClangScanDeps/modules-no-undeclared-includes.c
  clang/test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
  clang/test/Driver/stdc-predef.c
  clang/test/Driver/stdc-predef.i
  clang/unittests/Tooling/TestVisitor.h
  clang/unittests/libclang/LibclangTest.cpp

Index: clang/unittests/libclang/LibclangTest.cpp
===
--- clang/unittests/libclang/LibclangTest.cpp
+++ clang/unittests/libclang/LibclangTest.cpp
@@ -366,8 +366,11 @@
 "printf(\"mmm!!\");\n"
 "#endif");
 
-  ClangTU = clang_parseTranslationUnit(Index, Main.c_str(), nullptr, 0,
-   nullptr, 0, TUFlags);
+  const char *argv[] = {"-ffreestanding"};
+  const int argc = sizeof(argv) / sizeof(*argv);
+
+  ClangTU = clang_parseTranslationUnit(Index, Main.c_str(), argv, argc, nullptr,
+   0, TUFlags);
 
   CXSourceRangeList *Ranges = clang_getAllSkippedRanges(ClangTU);
   EXPECT_EQ(2U, Ranges->count);
@@ -607,6 +610,9 @@
 "printf(\"mmm!!\");\n"
 "#endif");
 
+  const char *argv[] = {"-ffreestanding"};
+  const int argc = sizeof(argv) / sizeof(*argv);
+
   for (int i = 0; i != 3; ++i) {
 unsigned flags = TUFlags | CXTranslationUnit_PrecompiledPreamble;
 if (i == 2)
@@ -616,7 +622,7 @@
clang_disposeTranslationUnit(ClangTU);  // dispose from previous iter
 
 // parse once
-ClangTU = clang_parseTranslationUnit(Index, Main.c_str(), nullptr, 0,
+ClangTU = clang_parseTranslationUnit(Index, Main.c_str(), argv, argc,
 

[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

[Tao Liang via Phabricator via cfe-commits]

Origami404 added a comment.

After digging more deeply into tests, I found that if we decide to make clang 
include `stdc-predef.h` or other things, the behavior of many 
libtooling-related tools (e.g. clang-tidy, clangd, and python binding) will be 
affected.

For example, `clang-tidy/checkers/cppcoreguidelines/macro-usage.cpp` tests 
improper macro usage. If we #include `stdc-predef.h` implicitly, it will give 
warnings to macros that are inside this header. Another example is 
`clang-tidy/checkers/portability/restrict-system-includes-disallow.cpp`, it 
will give errors for including `stdc-predef.h`.

In my opinion, we may need to spend more time discussing how to make 
`__STD_C_*` macros available. The way that D34158 
 take will affect too many tools using 
libtooling, which may not be a problem back in 2017.

By the way, I have made a patch to pass tests about clang-tidy and python 
binding by adding more `-ffreestanding` flags. But I think it's better to 
re-think the impact it will make instead of just making the test pass. But I 
still can not figure out why the `git clang-format` failed on the CI.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137043/new/

https://reviews.llvm.org/D137043

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137343: [clang] add -Wvla-stack-allocation

[Tao Liang via Phabricator via cfe-commits]

Origami404 updated this revision to Diff 483796.
Origami404 added a comment.

[clang] add -Wvla-stack-allocation and divide different VLA warning

New behaviors:

-Wvla-stack-allocation

  warns on use of a VLA that involves a stack allocation

-Wvla-portability

  warns on any use of a VM type, even if -Wvla-stack-allocation is
  given.

-Wvla

  groups together -Wvla-stack-allocation and -Wvla-portability

Fixes: https://github.com/llvm/llvm-project/issues/57098
Link: https://reviews.llvm.org/D132952

Co-authored-by: YingChi Long 

Differential Revision: https://reviews.llvm.org/D137343


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137343/new/

https://reviews.llvm.org/D137343

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/C/drs/dr3xx.c
  clang/test/Sema/cast.c
  clang/test/Sema/warn-vla.c
  clang/test/SemaCXX/warn-vla.cpp

Index: clang/test/SemaCXX/warn-vla.cpp
===
--- clang/test/SemaCXX/warn-vla.cpp
+++ clang/test/SemaCXX/warn-vla.cpp
@@ -1,7 +1,10 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wvla %s
 
 void test1(int n) { // expected-note {{here}}
-  int v[n]; // expected-warning {{variable length array}} expected-note {{parameter 'n'}}
+  int v[n]; /* expected-warning {{variable length array}} 
+   expected-note {{parameter 'n'}}
+   expected-warning {{variable length array that may require stack allocation used}}
+*/
 }
 
 void test2(int n, int v[n]) { // expected-warning {{variable length array}} expected-note {{parameter 'n'}} expected-note {{here}}
@@ -11,7 +14,10 @@
 
 template
 void test4(int n) { // expected-note {{here}}
-  int v[n]; // expected-warning {{variable length array}} expected-note {{parameter 'n'}}
+  int v[n]; /* expected-warning {{variable length array}} 
+   expected-note {{parameter 'n'}}
+   expected-warning {{variable length array that may require stack allocation used}}
+*/
 }
 
 template
Index: clang/test/Sema/warn-vla.c
===
--- clang/test/Sema/warn-vla.c
+++ clang/test/Sema/warn-vla.c
@@ -1,12 +1,59 @@
-// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -Wvla %s
-// RUN: %clang_cc1 -std=c89 -fsyntax-only -verify -Wvla %s
+// RUN: %clang_cc1 -std=c89 -fsyntax-only -verify=expected,c89 -Wvla %s
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=expected,c99 -Wvla %s
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=expected,only-stack-allocation -Wvla-stack-alloc -Wno-vla-portability %s
 
 void test1(int n) {
-  int v[n]; // expected-warning {{variable length array}}
+  int v[n]; /* 
+c89-warning {{variable length arrays are a C99 feature}}
+c89-warning {{variable length array that may require stack allocation used}}
+c99-warning {{variable length array used}}
+c99-warning {{variable length array that may require stack allocation used}}
+only-stack-allocation-warning {{variable length array that may require stack allocation used}}
+  */
 }
 
-void test2(int n, int v[n]) { // expected-warning {{variable length array}}
+void test2(int n, int v[n]) { /* 
+  c89-warning {{variable length arrays are a C99 feature}}
+  c99-warning {{variable length array used}}
+*/
 }
 
-void test3(int n, int v[n]); // expected-warning {{variable length array}}
+void test3(int n, int v[n]); /* 
+  c89-warning {{variable length arrays are a C99 feature}}
+  c99-warning {{variable length array used}}
+*/
 
+int test4_num;
+typedef int Test4[test4_num]; /* 
+  c89-warning {{variable length arrays are a C99 feature}} 
+  c99-warning {{variable length array used}}
+  expected-error {{variable length array declaration not allowed at file scope}}
+*/
+
+void func() {
+  typedef int test5[test4_num]; /* 
+c89-warning {{variable length arrays are a C99 feature}} 
+c99-warning {{variable length array used}} 
+  */
+
+  int test6[test4_num]; /*  
+c89-warning {{variable length arrays are a C99 feature}}
+c89-warning {{variable length array that may require stack allocation used}}
+c99-warning {{variable length array used}}
+c99-warning {{variable length array that may require stack allocation used}}
+only-stack-allocation-warning {{variable length array that may require stack allocation used}}
+  */
+
+  // FIXME: warn twice
+  (void)sizeof(int[test4_num]); /* 
+c89-warning {{variable length arrays are a C99 feature}}
+c89-warning {{variable length arrays are a C99 feature}}
+c99-warning {{variable length array used}}
+c99-warning {{variable length array used}}
+  */
+
+  int (*test7)[test4_num]; /*
+c89-warning {{variable length arrays are a C99 feature}}
+c99-warning {{variable length array used}}
+  */
+}
Index: clang/test/Sema/

[PATCH] D137343: [clang] add -Wvla-stack-allocation

[Tao Liang via Phabricator via cfe-commits]

Origami404 added a subscriber: mizvekov.
Origami404 added a comment.

Hello, I am back now and will be available anytime next week, so if anyone has 
any idea on this topic, please at me!

I haven't found a way that "feels right" to achieve consensus yet. But I do 
have something to share currently.

---

For the semantics of the warning flag, I agree with @uecker that 
`-Wvla-portability` should be changed to `-Wc++-compat`. In my reading of C2x 
standard, using VM type is acceptable and the only reason we need to issue a 
warning is that the C++ standard doesn't allow non-constant array length at 
all. But I have no opinion on the semantics of `-Wvla` and `-Wvla-stack-alloc`.

And should `-Wvla-stack-alloc` fires on static VLAs? (like 
`clang/test/drs/dr3xx.c:164`)

---

For implement details, I add a diag at ``, which will be called after a 
variable declaration. Here we can get the full type of the variable to tell 
whether the variable has a VLA type or just a VM type.

In the GitHub issue 
, 
@mizvekov said:

> A general check about any VLA usage at all should probably happen early when 
> we finished parsing and are building VariableArray types, probably in 
> `Sema::BuildArrayType`.
>
> Otherwise sprinkling around checks for VM types seems messy, so I don't think 
> considering VM for this problem is an efficient approach.

But AFAIK, a VLA type cannot be told just at `Sema::BuildArrayType` level 
because the constructing VLA type may just be a part of another VM type but not 
a VLA type for a VLA. At `Sema::BuildArrayType`, we seem only can detect VM 
type instead of VLA type.

---

In Ballman's summary, the `-Wvla-stack-alloc` should suppress the 
`-Wvla-portability`:

> -Wvla-portability warns on any use of a VM type, except if 
> -Wvla-stack-allocation is also enabled it won't warn on the use of VLA that 
> involves a stack allocation

I failed to achieve the "except" here because, in the current implementation, 
VM type-related warnings are checked directly at `Sema::BuildArrayType`, by 
`checkArraySize` which used `Sema::VerifyIntegerConstantExpression`. But the 
`Sema::VerifyIntegerConstantExpression` requires at least one diag at each 
non-ICE occurrence. I haven't found a way to avoid the diag yet.

---

By the way, I seem to find a bug in the current implementation of VM type 
warning. For VLA at sizeof context, the warning will be issued twice. Github 
issue here . It cause the 
test `clang/test/Sema/warn-vla.c:48` giving two warnings.

---

The reason why I set both `-Wvla-stack-alloc` and `-Wvla-portability` to 
`DefaultIgnore` is that:

- if both of them are not `DefaultIgnore`, 200 tests will fail
- if only `-Wvla-stack-alloc` is not `DefaultIgnore`, 169 tests will fail
- if both of them are `DefaultIgnore`, only 4 tests will fail

Most of the failed tests seem cannot be avoided because they just use VLAs. If 
`DefaultIgnore` is strongly unrecommended, I can go through all the tests and 
add an expected warning diag for them. It's a bit time-consuming, but I am free 
all day now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137343/new/

https://reviews.llvm.org/D137343

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137343: [clang] add -Wvla-stack-allocation

[Tao Liang via Phabricator via cfe-commits]

Origami404 created this revision.
Herald added a project: All.
Origami404 edited the summary of this revision.
Origami404 edited the summary of this revision.
Origami404 added reviewers: aaron.ballman, efriedma.
Origami404 added a subscriber: inclyc.
Origami404 published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

New behaviors:

`-Wvla`: produces portability warning for all VLAs like before
`-Wvla-portability`: not exists (equal to `-Wvla`)
`-Wvla-stack-allocation -Wno-vla`: only warns for VLAs that need stack 
allocation
`-Wvla -Wvla-stack-allocation`: warn for VLAs that need stack allocation, and 
give portablility warnings for other VLAs

Note that only one warning can be produced by one VLA because of
implementation complexity. Currently, VLAs are detected in
`BuildArrayType`, which will be used both by Parser and Tree Transformer.
And it passes the VLA-related warnings as an argument to other
functions. Producing multiple warnings for one VLA needs to change many
functions.

Fixes: https://github.com/llvm/llvm-project/issues/57098
Link: https://reviews.llvm.org/D132952

Co-authored-by: YingChi Long 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137343

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/warn-vla.c

Index: clang/test/Sema/warn-vla.c
===
--- clang/test/Sema/warn-vla.c
+++ clang/test/Sema/warn-vla.c
@@ -1,12 +1,39 @@
-// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -Wvla %s
-// RUN: %clang_cc1 -std=c89 -fsyntax-only -verify -Wvla %s
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=expected,no-fallback-c99 -Wvla -Wvla-stack-allocation %s
+// RUN: %clang_cc1 -std=c89 -fsyntax-only -verify=expected,no-fallback-c89 -Wvla -Wvla-stack-allocation %s
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=expected,fallback -Wvla %s
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=expected,only-stack-allocation -Wvla-stack-allocation -Wno-vla %s
 
 void test1(int n) {
-  int v[n]; // expected-warning {{variable length array}}
+  int v[n]; /* no-fallback-c89-warning {{variable length arrays are a C99 feature}}
+   no-fallback-c99-warning {{variable length array that may require stack allocation used}}
+   fallback-warning {{variable length array used}}
+   only-stack-allocation-warning {{variable length array that may require stack allocation used}}
+*/
 }
 
-void test2(int n, int v[n]) { // expected-warning {{variable length array}}
+void test2(int n, int v[n]) { /* no-fallback-c89-warning {{variable length arrays are a C99 feature}}
+ no-fallback-c99-warning {{variable length array used}}
+ fallback-warning {{variable length array used}}
+  */
 }
 
-void test3(int n, int v[n]); // expected-warning {{variable length array}}
+void test3(int n, int v[n]); /* no-fallback-c89-warning {{variable length arrays are a C99 feature}}
+no-fallback-c99-warning {{variable length array used}}
+fallback-warning {{variable length array used}}
+  */
 
+int test4_num;
+typedef int Test4[test4_num]; /* no-fallback-c89-warning {{variable length arrays are a C99 feature}} 
+ no-fallback-c99-warning {{variable length array used}}
+ fallback-warning {{variable length array used}}
+ expected-error {{variable length array declaration not allowed at file scope}}
+  */
+
+void test5() {
+  typedef int type[test4_num]; /* no-fallback-c89-warning {{variable length arrays are a C99 feature}} 
+  no-fallback-c99-warning {{variable length array that may require stack allocation used}}
+  fallback-warning {{variable length array used}}
+  only-stack-allocation-warning {{variable length array that may require stack allocation used}}
+*/
+
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeLocVisitor.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
@@ -2530,7 +2531,8 @@
 return QualType();
   }
 
-  // VLAs always produce at least a -Wvla diagnostic, sometimes an error.
+  // VLAs always produce at least a -Wvla-portability or -Wvla-stack-allocation
+  // diagnostic, sometimes an 

[PATCH] D137343: [clang] add -Wvla-stack-allocation

[Tao Liang via Phabricator via cfe-commits]

Origami404 added a comment.

ping :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137343/new/

https://reviews.llvm.org/D137343

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

[Tao Liang via Phabricator via cfe-commits]

Origami404 added a comment.

ping :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137043/new/

https://reviews.llvm.org/D137043

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

[Tao Liang via Phabricator via cfe-commits]

Origami404 added a comment.

In D137043#3935129 , @aaronpuchert 
wrote:

> This include-if-exists mechanism seems brittle to me.

Do you mean the way that we used to test a file and include it (inserting `#if 
__has_include`) is brittle or compilation flags like `--include-if-exists` 
themselves are?

> Can we not make it dependent on the triple, i.e. include the file if we're 
> using a libc implementation that's known to provide (and require) this file?

I am afarid that without depending on triples, we are not able to distinguish 
what environment we are in or libc we use.  Can you explain more about this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137043/new/

https://reviews.llvm.org/D137043

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

[Tao Liang via Phabricator via cfe-commits]

Origami404 updated this revision to Diff 477453.
Origami404 added a comment.

Change implements according reviews.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137043/new/

https://reviews.llvm.org/D137043

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
  clang/test/Driver/stdc-predef.c
  clang/test/Driver/stdc-predef.i

Index: clang/test/Driver/stdc-predef.i
===
--- /dev/null
+++ clang/test/Driver/stdc-predef.i
@@ -0,0 +1,9 @@
+// The automatic preinclude of stdc-predef.h should not occur if
+// the source filename indicates a preprocessed file.
+//
+// RUN: %clang %s -### -c 2>&1 \
+// RUN: --sysroot=%S/Inputs/stdc-predef \
+// RUN: -target x86_64-unknown-linux-musl \
+// RUN: | FileCheck --implicit-check-not "stdc-predef.h" %s
+
+int i;
Index: clang/test/Driver/stdc-predef.c
===
--- /dev/null
+++ clang/test/Driver/stdc-predef.c
@@ -0,0 +1,61 @@
+// Test that clang preincludes stdc-predef.h, if we are using libc that does not
+// pre-include it, e.g. Musl
+
+// Musl-based system need this addition include.
+//
+// RUN: %clang %s -### -c 2>&1 \
+// RUN: -target x86_64-unknown-linux-musl \
+// RUN: --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck -check-prefix CHECK-CPP-FLAG %s
+
+// Gnu-based system does not need this addition include.
+//
+// RUN: %clang %s -### -c -ffreestanding 2>&1 \
+// RUN: -target x86_64-unknown-linux-gnu \
+// RUN: --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck --implicit-check-not "stdc-predef.h" %s
+
+// Standalone system does not need this addition include.
+//
+// RUN: %clang %s -### -c 2>&1 \
+// RUN: -target x86_64-unknown-linux-musl -ffreestanding \
+// RUN: --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck --implicit-check-not "stdc-predef.h" %s
+
+// Because this behavior is implemented by adding preprocessor flags in the 
+// driver, so if we disabled preprocess, the include flag should not appear.
+//
+// RUN: %clang -x cpp-output %s -### -c 2>&1 \
+// RUN: -target x86_64-unknown-linux-musl \
+// RUN: --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck --implicit-check-not "stdc-predef.h" %s
+
+// This behavior should appear in all languages that use the c preprocessor,
+// including C, C++ and Objective-C, as long as the system uses musl.
+//
+// RUN: %clang -x objective-c %s -### -c 2>&1 \
+// RUN: -target x86_64-unknown-linux-musl \
+// RUN: --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck -check-prefix CHECK-CPP-FLAG %s
+
+// If a musl-based system does not have this header, give an error at line 1.
+//
+// RUN: %clang %s -c -Xclang -verify=expected 2>&1 \
+// RUN: -target x86_64-unknown-linux-musl \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree 
+// expected-error@1 {{'stdc-predef.h' file not found}}
+
+// Check if the file is really included by macro.
+//
+// RUN: %clang %s -c -Xclang -verify=ok -DCHECK_DUMMY=1 \
+// RUN: -target x86_64-unknown-linux-musl \
+// RUN: --sysroot=%S/Inputs/stdc-predef 
+// ok-no-diagnostics
+
+// CHECK-CPP-FLAG: "-include" "stdc-predef.h"
+int i;
+#if CHECK_DUMMY
+#if !DUMMY_STDC_PREDEF 
+  #error "Expected macro symbol DUMMY_STDC_PREDEF is not defined."
+#endif
+#endif
Index: clang/test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
===
--- /dev/null
+++ clang/test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
@@ -0,0 +1,4 @@
+#ifndef	_STDC_PREDEF_H
+#define	_STDC_PREDEF_H	1
+#define DUMMY_STDC_PREDEF 1
+#endif
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -636,6 +636,12 @@
 
   if (!DriverArgs.hasArg(options::OPT_nobuiltininc) && getTriple().isMusl())
 addSystemInclude(DriverArgs, CC1Args, ResourceDirInclude);
+
+  // add implict include for musl-based non-free-standing system
+  if (!DriverArgs.hasArg(options::OPT_ffreestanding) && getTriple().isMusl()) {
+CC1Args.push_back("-include");
+CC1Args.push_back("stdc-predef.h");
+  }
 }
 
 void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -398,6 +398,8 @@
   PCH or modules. When Clang hits this limit, it now produces notes mentioning
   which header and source files are consuming large amounts of this space.
   ``#pragma clang __debug sloc_usage`` can also be used to request this report.
+- Clang will implicitly add ``#include "stdc-predef.h"`` on musl-based system 
+  likes GCC.
 
 Non-comprehensive list of changes in this release
 

[PATCH] D137343: [clang] add -Wvla-stack-allocation

[Tao Liang via Phabricator via cfe-commits]

Origami404 updated this revision to Diff 477482.
Origami404 added a comment.

Update release notes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137343/new/

https://reviews.llvm.org/D137343

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/warn-vla.c

Index: clang/test/Sema/warn-vla.c
===
--- clang/test/Sema/warn-vla.c
+++ clang/test/Sema/warn-vla.c
@@ -1,12 +1,39 @@
-// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -Wvla %s
-// RUN: %clang_cc1 -std=c89 -fsyntax-only -verify -Wvla %s
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=expected,no-fallback-c99 -Wvla -Wvla-stack-allocation %s
+// RUN: %clang_cc1 -std=c89 -fsyntax-only -verify=expected,no-fallback-c89 -Wvla -Wvla-stack-allocation %s
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=expected,fallback -Wvla %s
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=expected,only-stack-allocation -Wvla-stack-allocation -Wno-vla %s
 
 void test1(int n) {
-  int v[n]; // expected-warning {{variable length array}}
+  int v[n]; /* no-fallback-c89-warning {{variable length arrays are a C99 feature}}
+   no-fallback-c99-warning {{variable length array that may require stack allocation used}}
+   fallback-warning {{variable length array used}}
+   only-stack-allocation-warning {{variable length array that may require stack allocation used}}
+*/
 }
 
-void test2(int n, int v[n]) { // expected-warning {{variable length array}}
+void test2(int n, int v[n]) { /* no-fallback-c89-warning {{variable length arrays are a C99 feature}}
+ no-fallback-c99-warning {{variable length array used}}
+ fallback-warning {{variable length array used}}
+  */
 }
 
-void test3(int n, int v[n]); // expected-warning {{variable length array}}
+void test3(int n, int v[n]); /* no-fallback-c89-warning {{variable length arrays are a C99 feature}}
+no-fallback-c99-warning {{variable length array used}}
+fallback-warning {{variable length array used}}
+  */
 
+int test4_num;
+typedef int Test4[test4_num]; /* no-fallback-c89-warning {{variable length arrays are a C99 feature}} 
+ no-fallback-c99-warning {{variable length array used}}
+ fallback-warning {{variable length array used}}
+ expected-error {{variable length array declaration not allowed at file scope}}
+  */
+
+void test5() {
+  typedef int type[test4_num]; /* no-fallback-c89-warning {{variable length arrays are a C99 feature}} 
+  no-fallback-c99-warning {{variable length array that may require stack allocation used}}
+  fallback-warning {{variable length array used}}
+  only-stack-allocation-warning {{variable length array that may require stack allocation used}}
+*/
+
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeLocVisitor.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
@@ -2530,7 +2531,8 @@
 return QualType();
   }
 
-  // VLAs always produce at least a -Wvla diagnostic, sometimes an error.
+  // VLAs always produce at least a -Wvla-portability or -Wvla-stack-allocation
+  // diagnostic, sometimes an error.
   unsigned VLADiag;
   bool VLAIsError;
   if (getLangOpts().OpenCL) {
@@ -2538,7 +2540,26 @@
 VLADiag = diag::err_opencl_vla;
 VLAIsError = true;
   } else if (getLangOpts().C99) {
-VLADiag = diag::warn_vla_used;
+// after C99, VLA is no longer an extension.
+if (getCurScope()->getFlags() == Scope::ScopeFlags::DeclScope) {
+  // VLA in file scope typedef will have an error, and should not have a
+  // warning of portability. But for backward compatibility, we preform the
+  // exact same action like before (which will give an warning of "vla
+  // used").
+  VLADiag = diag::warn_vla_portability;
+} else if (getCurScope()->isFunctionPrototypeScope()) {
+  // VLA in function prototype is acceptable by C2x standard
+  // so just give a protability warning
+  VLADiag = diag::warn_vla_portability;
+} else {
+  // in other case, a VLA will cause stack allocation
+  // if -Wvla-stack-allocation is

[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

[Tao Liang via Phabricator via cfe-commits]

Origami404 updated this revision to Diff 478977.
Origami404 added a comment.

Improved comments and release notes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137043/new/

https://reviews.llvm.org/D137043

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
  clang/test/Driver/stdc-predef.c
  clang/test/Driver/stdc-predef.i

Index: clang/test/Driver/stdc-predef.i
===
--- /dev/null
+++ clang/test/Driver/stdc-predef.i
@@ -0,0 +1,9 @@
+// The automatic preinclude of stdc-predef.h should not occur if
+// the source filename indicates a preprocessed file.
+//
+// RUN: %clang %s -### -c 2>&1 \
+// RUN: --sysroot=%S/Inputs/stdc-predef \
+// RUN: -target x86_64-unknown-linux-musl \
+// RUN: | FileCheck --implicit-check-not "stdc-predef.h" %s
+
+int i;
Index: clang/test/Driver/stdc-predef.c
===
--- /dev/null
+++ clang/test/Driver/stdc-predef.c
@@ -0,0 +1,62 @@
+// Test that clang preincludes stdc-predef.h, if we are using libc that does not
+// pre-include it, e.g. Musl
+
+// Musl-based systems need this addition include.
+//
+// RUN: %clang %s -### -c 2>&1 \
+// RUN: -target x86_64-unknown-linux-musl \
+// RUN: --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck -check-prefix CHECK-CPP-FLAG %s
+
+// GNU-based systems do not need this addition include.
+//
+// RUN: %clang %s -### -c -ffreestanding 2>&1 \
+// RUN: -target x86_64-unknown-linux-gnu \
+// RUN: --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck --implicit-check-not "stdc-predef.h" %s
+
+// Freestanding compilations do not need this addition include.
+//
+// RUN: %clang %s -### -c 2>&1 \
+// RUN: -target x86_64-unknown-linux-musl -ffreestanding \
+// RUN: --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck --implicit-check-not "stdc-predef.h" %s
+
+// Because this behavior is implemented by adding preprocessor flags in the 
+// driver, disabling preprocessing means the include flag should not appear.
+//
+// RUN: %clang -x cpp-output %s -### -c 2>&1 \
+// RUN: -target x86_64-unknown-linux-musl \
+// RUN: --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck --implicit-check-not "stdc-predef.h" %s
+
+// This behavior should appear in all files clang accepted that use the C 
+// preprocessor, including C, C++ and Objective-C files, as long as the system 
+// uses musl.
+//
+// RUN: %clang -x objective-c %s -### -c 2>&1 \
+// RUN: -target x86_64-unknown-linux-musl \
+// RUN: --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck -check-prefix CHECK-CPP-FLAG %s
+
+// If a musl-based system does not have this header, give an error at line 1.
+//
+// RUN: %clang %s -c -Xclang -verify=expected 2>&1 \
+// RUN: -target x86_64-unknown-linux-musl \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree 
+// expected-error@1 {{'stdc-predef.h' file not found}}
+
+// Check if the file is really included by macro.
+//
+// RUN: %clang %s -c -Xclang -verify=ok -DCHECK_DUMMY=1 \
+// RUN: -target x86_64-unknown-linux-musl \
+// RUN: --sysroot=%S/Inputs/stdc-predef 
+// ok-no-diagnostics
+
+// CHECK-CPP-FLAG: "-include" "stdc-predef.h"
+int i;
+#if CHECK_DUMMY
+#if !DUMMY_STDC_PREDEF 
+  #error "Expected macro symbol DUMMY_STDC_PREDEF is not defined."
+#endif
+#endif
Index: clang/test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
===
--- /dev/null
+++ clang/test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
@@ -0,0 +1,4 @@
+#ifndef	_STDC_PREDEF_H
+#define	_STDC_PREDEF_H	1
+#define DUMMY_STDC_PREDEF 1
+#endif
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -636,6 +636,12 @@
 
   if (!DriverArgs.hasArg(options::OPT_nobuiltininc) && getTriple().isMusl())
 addSystemInclude(DriverArgs, CC1Args, ResourceDirInclude);
+
+  // Add an implicit include for musl-based non-free-standing systems.
+  if (!DriverArgs.hasArg(options::OPT_ffreestanding) && getTriple().isMusl()) {
+CC1Args.push_back("-include");
+CC1Args.push_back("stdc-predef.h");
+  }
 }
 
 void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -398,6 +398,8 @@
   PCH or modules. When Clang hits this limit, it now produces notes mentioning
   which header and source files are consuming large amounts of this space.
   ``#pragma clang __debug sloc_usage`` can also be used to request this report.
+- Clang will implicitly add ``#include "stdc-predef.h"`` on musl-based systems
+  as GCC does.
 
 Non-comprehensive list of changes in this relea

[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

[Tao Liang via Phabricator via cfe-commits]

Origami404 marked 7 inline comments as done.
Origami404 added inline comments.



Comment at: clang/test/Driver/stdc-predef.c:33-34
+
+// This behavior should appear in all languages that use the c preprocessor,
+// including C, C++ and Objective-C, as long as the system uses musl.
+//

aaron.ballman wrote:
> What about Fortran?
Only files that using clang to compile will be affected. I have updated my 
expression here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137043/new/

https://reviews.llvm.org/D137043

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137343: [clang] add -Wvla-stack-allocation

[Tao Liang via Phabricator via cfe-commits]

Origami404 added a comment.

>   int foo(void);
>   
>   void bar(int a, int b[*]); // variable length array used, correct
>   void bar(int a, int b[a]) { variable length array used, correct
> int x[foo()]; // variable length array that may require stack allocation 
> used, correct
>   
> (void)sizeof(int[foo()]); // variable length array that may require stack 
> allocation used, incorrect and the diagnostic triggers twice
>   
> int (*y)[foo()]; // variable length array that may require stack 
> allocation used, incorrect, this is a pointer to a VLA
>   }

Thanks to ballman's example here, it remains me that when non-LCE appears as an 
array index in a variable declaration, it is still possible to be a part of a 
pointer/function prototype instead of a VLA. Classifying different VLA usages 
requires more information about the declaration.

The old VLA checking implements just insert checking for the location in array 
type construction when a non-ICE is used as the array's length. And it assumes 
that only one error or warning could be produced by one VLA usage. So the old 
method seems hard to be extended to support the new checking on VLAs.

I plan to throw away the old method here, trying to use "eval functions" on AST 
of declarations or LLVM IR to classify different kinds of VLAs.

Since I am preparing for several final examinations next week, I will try to 
give a new implementation in about a week. And in advance, sorry for my late 
response next week.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137343/new/

https://reviews.llvm.org/D137343

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits