aaron.ballman added inline comments.
================ Comment at: test/Sema/dllexport-1.c:1 +// RUN: %clang_cc1 -triple i686-win32 -emit-llvm -fms-extensions -std=c99 < %s| FileCheck %s +// RUN: %clang_cc1 -triple x86_64-win32 -emit-llvm -fms-extensions -std=c11 < %s | FileCheck %s ---------------- aaron.ballman wrote: > This test should live in CodeGen not Sema. Do we need this many RUN lines? Also, why the different language modes? Also, do you need to use `<` in the command? ================ Comment at: test/Sema/dllexport-1.c:8 + +// CHECK: @y = common dso_local dllexport global i32 0, align 4 + ---------------- zahiraam wrote: > aaron.ballman wrote: > > Are x and z also exported as expected? > Only x and y are exported. > > > **@x = dso_local dllexport constant i32 3, align 4** > @z = dso_local constant i32 4, align 4 > **@y = common dso_local dllexport global i32 0, align 4** > > But then if I take this simple case: > extern int const z = 4; > > int main() { > int a = z + 2; > return a; > } > ksh-3.2$ clang -c test3.c > test3.c:1:18: warning: 'extern' variable has an initializer > [-Wextern-initializer] > extern int const z = 4; > ^ > 1 warning generated. > ksh-3.2$ dumpbin /symbols test3.o | grep External > 00F 00000000 SECT1 notype () External | main > **010 00000000 SECT5 notype External | z** > ksh-3.2$ > When emitting the IR, z is declared as a local constant (not exported): > @z = dso_local constant i32 4, align 4 > > Okay, please add the expected CHECK lines for `x` and `z` to the test. Thank you for clarifying! ================ Comment at: test/Sema/dllexport-1.cpp:2 +// RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s +// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify -std=c++11 %s + ---------------- I don't think we need this RUN line. I don't know if we need the triple in the previous RUN line either. ================ Comment at: test/Sema/dllexport-1.cpp:4 + +// CHECK: @"?x@@3HB" = dso_local dllexport constant i32 3, align 4 + ---------------- aaron.ballman wrote: > This test has no FileCheck line. > > The goal here is for the tests in Sema to test the semantic behavior (that > warnings are issued when expected and not issued when not expected) and to > add tests in CodeGen to test the generated code (are things properly > exported, are things we don't expect to be exported not exported, etc). I > think you should split your tests accordingly rather than try to verify too > much at once. This CHECK line still has no impact; I think it should be removed from the test. ================ Comment at: test/Sema/dllexport-2.cpp:2 +// RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s +// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify -std=c++11 %s + ---------------- I don't think we need this RUN line. I don't know if we need the triple in the previous RUN line either. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits