[PATCH] D131683: Diagnosing the Future Keywords

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/TokenKinds.def:384
+C2X_KEYWORD(true                        , BOOLSUPPORT)
+C2X_KEYWORD(remove_quals                , KEYC2X)
+
----------------
Codesbyusman wrote:
> aaron.ballman wrote:
> > This is technically correct, but I think we should remove it until we go to implement that paper instead of introducing the keyword out of thin air here.
> > 
> > Btw, I think that should be `, 0` instead of `, KEYC2X` given the use of `C2X_KEYWORD`, right?
> > This is technically correct, but I think we should remove it until we go to implement that paper instead of introducing the keyword out of thin air here.
> > 
> > Btw, I think that should be `, 0` instead of `, KEYC2X` given the use of `C2X_KEYWORD`, right?
> 
> Don't know much but what I understand that this is a keyword but not published officially by language but will be in future. so I think will be good to remove it or can add Fixme?
> Don't know much but what I understand that this is a keyword but not published officially by language but will be in future. so I think will be good to remove it or can add Fixme?

Close -- the issue is more that we have no support for `remove_quals` in the compiler and it's easier for someone to add the keyword when implementing that feature. So you should remove the line entirely -- someone else will add it in the future.


================
Comment at: clang/lib/Basic/IdentifierTable.cpp:852
+
+//===----------------------------------------------------------------------===//
+// IdentifierTable Implementation
----------------
erichkeane wrote:
> This comment block shouldn't be here at all.
It looks like you might have missed this comment.


================
Comment at: clang/lib/Basic/IdentifierTable.cpp:863
+
+  // Getting the flag value. i.e bool keyword passing it name to the function
+  // and will return the flag value that which keys are used then will use for
----------------
erichkeane wrote:
> This comment doesn't really make any sense here.  I dont understand what you're trying to say.
It looks like you might have missed this comment (it's about `Checking the Language mode and then for the diagnostics.`)


================
Comment at: clang/test/Parser/static_assert.c:1
-// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -verify=c2x %s
-// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -fms-compatibility -verify=c2x-ms %s
+// RUN: %clang_cc1 -fsyntax-only -std=c17 -DTEST_SPELLING -Weverything -verify=c17 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c17 -DTEST_SPELLING -fms-compatibility -verify=c17-ms %s
----------------
Why did you add `-Weverything` here? That seems odd.


================
Comment at: clang/test/Parser/static_assert.c:23
+                      // c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}} \
+                      // c17-warning {{ function declaration without a prototype is deprecated in all versions of C}} \
+                      // c17-ms-warning {{use of 'static_assert' without inclusion of <assert.h> is a Microsoft extension}} 
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131683