nickdesaulniers added a comment.
Created: https://reviews.llvm.org/D52849
Repository:
rC Clang
https://reviews.llvm.org/D52248
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
nickdesaulniers abandoned this revision.
nickdesaulniers added a comment.
Well that didn't do what I wanted...(sorry, still learning arc). Going to
abandon this and push a much simpler fix.
Repository:
rC Clang
https://reviews.llvm.org/D52248
nickdesaulniers updated this revision to Diff 168183.
nickdesaulniers added a comment.
[SEMA] split ExtWarn dupl-decl-spec's into Extension and ExtWarn
For types deduced from typedef's and typeof's, don't warn for duplicate
declaration specifiers in C90 unless -pedantic.
Create a third
nickdesaulniers added inline comments.
Comment at: lib/Sema/DeclSpec.cpp:441-442
else
-DiagID = IsExtension ? diag::ext_duplicate_declspec :
- diag::warn_duplicate_declspec;
+DiagID = IsExtension ? diag::ext_duplicate_declspec
+
rsmith added a comment.
Downgrading the C89 warning for duplicate qualifiers from typedefs / __typeof
from `ExtWarn` to `Extension` seems very reasonable to me. However, I don't see
a justification for removing the warning for duplicate explicit (non-typedef)
qualifiers in C99, nor for
nickdesaulniers updated this revision to Diff 167845.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.
- move back untouched test case
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
include/clang/Basic/DiagnosticSemaKinds.td
nickdesaulniers updated this revision to Diff 167843.
nickdesaulniers marked 2 inline comments as done.
nickdesaulniers added a comment.
- add back test for typedef typedef
- add test for dupl _Atomic and restrict
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
nickdesaulniers marked an inline comment as done.
nickdesaulniers added inline comments.
Comment at: test/FixIt/fixit.c:50
-// CHECK: const typedef int int_t;
-const typedef typedef int int_t; // expected-warning {{duplicate 'typedef'}}
-
nickdesaulniers wrote:
nickdesaulniers added inline comments.
Comment at: test/FixIt/fixit.c:50
-// CHECK: const typedef int int_t;
-const typedef typedef int int_t; // expected-warning {{duplicate 'typedef'}}
-
Ah, this was a case I should add one last test for. I think reviewers
nickdesaulniers added a comment.
Ok, I think this is ready for rereview.
Comment at: test/Parser/atomic.c:5
typedef _Atomic int atomic_int;
-typedef _Atomic _Atomic _Atomic(int) atomic_int; // expected-warning
{{duplicate '_Atomic' declaration specifier}}
nickdesaulniers updated this revision to Diff 167565.
nickdesaulniers added a comment.
Herald added a subscriber: jfb.
- split duplicate_declspec into Extension and Extwarn from Extwarn.
- rename ext_duplicate_declspec to be the Extension, not the Extwarn.
- Fix C++ case.
- Use tablegen'd
rsmith added a comment.
The right way to produce diagnostics under pedantic mode is to model them as
Extension or ExtWarn in the .td file, not by checking the Pedantic diagnostic
option directly. If this is an intentional GNU extension too, that makes things
a bit more complex (one approach
nickdesaulniers added a comment.
> Should GCC instead warn for the typedef case for -std=c89 (non pedantic),
> according to C90 6.5.3?
Seems like yes: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80868#c6
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87435
But let's wait a bit to see what the
nickdesaulniers added a comment.
This now matches GCC AFAICT. My only question is:
Should GCC instead warn for the typedef case for -std=c89 (non pedantic),
according to C90 6.5.3?
Repository:
rC Clang
https://reviews.llvm.org/D52248
___
nickdesaulniers updated this revision to Diff 166743.
nickdesaulniers added a comment.
- adjust wording in comment
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89-const.c
Index: test/Sema/gnu89-const.c
nickdesaulniers updated this revision to Diff 166742.
nickdesaulniers added a comment.
- fix typo s/CNU/GNU/g and update NOTEs
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89-const.c
Index: test/Sema/gnu89-const.c
nickdesaulniers added inline comments.
Comment at: test/Sema/gnu89-const.c:41-46
+CHECK-CNU99-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU99-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration
specifier
+CHECK-CNU11-NOT: 27:1: warning: duplicate
nickdesaulniers updated this revision to Diff 166740.
nickdesaulniers added a comment.
- condense CHECK-prefixes into CHECK for const const
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89-const.c
Index: test/Sema/gnu89-const.c
nickdesaulniers updated this revision to Diff 166736.
nickdesaulniers added a comment.
- add ISO C tests, handle typedef case new tests found
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89-const.c
Index: test/Sema/gnu89-const.c
nickdesaulniers updated this revision to Diff 166737.
nickdesaulniers added a comment.
- remove debug statments
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89-const.c
Index: test/Sema/gnu89-const.c
nickdesaulniers added inline comments.
Comment at: test/Sema/gnu89.c:1-2
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s
nickdesaulniers added inline comments.
Comment at: lib/Sema/SemaType.cpp:1682
// or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
srhines wrote:
> This is broken
nickdesaulniers updated this revision to Diff 166578.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.
- also run test on gnu99+
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89-const.c
Index:
nickdesaulniers updated this revision to Diff 166351.
nickdesaulniers added a comment.
- warn only if -pedantic, add gcc bug test cases
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89.c
Index: test/Sema/gnu89.c
nickdesaulniers added inline comments.
Comment at: test/Sema/gnu89.c:1-2
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s
nickdesaulniers added inline comments.
Comment at: test/Sema/gnu89.c:1-2
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s
srhines added inline comments.
Comment at: lib/Sema/SemaType.cpp:1682
// or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
This is broken for C11 and C17 (even
nickdesaulniers updated this revision to Diff 166357.
nickdesaulniers added a comment.
- add line numbers to match specific warning lines
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89-const.c
Index: test/Sema/gnu89-const.c
nickdesaulniers updated this revision to Diff 166355.
nickdesaulniers added a comment.
- move test to new file, use check-prefix for both cases
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89-const.c
Index: test/Sema/gnu89-const.c
lebedev.ri added inline comments.
Comment at: test/Sema/gnu89.c:1-2
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89
manojgupta added a comment.
lgtm. But someone more familiar with these code paths should approve.
Repository:
rC Clang
https://reviews.llvm.org/D52248
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
nickdesaulniers updated this revision to Diff 166352.
nickdesaulniers added a comment.
- git-clang-format HEAD~
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89.c
Index: test/Sema/gnu89.c
manojgupta added a comment.
As per https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80868, I thought GCC also
emits this error but only with -pedantic. So probably should keep this error
but under -Wextra or another appropriate group?
Repository:
rC Clang
https://reviews.llvm.org/D52248
joel added a comment.
I built Linux using this patch applied to trunk and it worked for me. Thanks!
Repository:
rC Clang
https://reviews.llvm.org/D52248
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
nickdesaulniers updated this revision to Diff 166035.
nickdesaulniers added a comment.
git-clang-format HEAD~
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89.c
Index: test/Sema/gnu89.c
nickdesaulniers created this revision.
nickdesaulniers added reviewers: srhines, gbiv.
Herald added a reviewer: george.burgess.iv.
Herald added a subscriber: cfe-commits.
nickdesaulniers removed a reviewer: gbiv.
Fixes PR32985.
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
36 matches
Mail list logo
