[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@nickdesaulniers
Thank you for your review!

I do not have permission to land this patch, so could you please do it on my 
behalf?
Here is my information:
Name: `Ken Matsui`
Email: `v...@kmatsui.me`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Sorry for having missed it. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 429877.
ken-matsui added a comment.

Remove `- < %s`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.S


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Removed it 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 429872.
ken-matsui added a comment.

Remove `-x assembler-with-cpp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.S


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s - < %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s - < %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:484
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.

nickdesaulniers wrote:
> Also, IIRC the token used to start a comment in assembler differs per 
> architecture. This might be the simplest fix, for now.
Ah, I did not know that. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you for your review! I've fixed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 429869.
ken-matsui added a comment.

Update the code as reviewed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.S


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp - < %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp - < %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

I see. Thank you. I fixed the issue using its workaround.

https://reviews.llvm.org/D125727


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D125727: Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui created this revision.
ken-matsui added a reviewer: aaron.ballman.
Herald added a project: All.
ken-matsui requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch is itended to avoid suggesting typoed directives in `.S` files to 
support the cases of `#` directives treated as comments or various pseudo-ops. 
The feature is implemented in https://reviews.llvm.org/D124726. Fixes: 
https://reviews.llvm.org/D124726#3516346.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125727

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.S


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@nathanchance

Oops, thank you for your comment!
I would like to work on a hotfix patch for this issue.

@aaron.ballman

Should we entirely opt-out of this suggestion on `.S` files? I think there are 
other approaches here, such as decreasing the max edit distance to avoid 
suggesting this case in `.S` files, but this approach is avoiding just this 
edge case so that it would possibly bring a new issue like this. Conversely, 
opting out of this suggestion on the whole `.S` files will not be able to catch 
any typoes. Considering possible edge cases (`#` directives are also treated as 
comments), I think opting out would be the only way, unfortunately. What do you 
think?

For now, I will create a patch opting out of this suggestion on `.S` files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you for your support!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@aaron.ballman

I prepared a new public email address: `v...@kmatsui.me` instead of the 
auto-generated one.
So, could you please use the new one for future commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@aaron.ballman

I prepared a new public email address: `g...@kmatsui.me` instead of the 
auto-generated one.
So, could you please use the new one for future commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 429005.
ken-matsui added a comment.

Revert the changes for errored directives


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c

Index: clang/test/Preprocessor/suggest-typoed-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.c
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=pre-c2x-cpp2b %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=c2x-cpp2b %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=c2x-cpp2b %s
+
+// id:pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   not suggested to '#elifdef'
+// elfindef:  not suggested to '#elifdef'
+// elfinndef: not suggested to '#elifndef'
+// els:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#endif'?}}
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+// id:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifndef'?}}
+// els:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#endif'?}}
+
+#ifdef UNDEFINED
+#i // no diagnostic
+#endif
+
+#if special_compiler
+#special_compiler_directive // no diagnostic
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -266,6 +266,51 @@
 .Default(false);
 }
 
+/// Find a similar string in `Candidates`.
+///
+/// \param LHS a string for a similar string in `Candidates`
+///
+/// \param Candidates the candidates to find a similar string.
+///
+/// \returns a similar string if exists. If no similar string exists,
+/// returns None.
+static Optional findSimilarStr(
+StringRef LHS, const std::vector ) {
+  // We need to check if `Candidates` has the exact case-insensitive string
+  // because the Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (LHS.equals_insensitive(C)) {
+  return C;
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If the LHS size is less than 3, use the LHS size minus 1 and if not,
+  // use the LHS size divided by 3.
+  size_t Length = LHS.size();
+  size_t MaxDist = Length < 3 ? Length - 1 : Length / 3;
+
+  Optional> SimilarStr = None;
+  for (StringRef C : Candidates) {
+size_t CurDist = LHS.edit_distance(C, true);
+if (CurDist <= MaxDist) {
+  if (!SimilarStr.hasValue()) {
+// The first similar string found.
+SimilarStr = {C, CurDist};
+  } else if (CurDist < SimilarStr->second) {
+// More similar string found.
+SimilarStr = {C, CurDist};
+  }
+}
+  }
+
+  if (SimilarStr.hasValue()) {
+return SimilarStr->first;
+  } else {
+return None;
+  }
+}
+
 bool Preprocessor::CheckMacroName(Token , MacroUse isDefineUndef,
   bool *ShadowFlag) {
   // Missing macro name?
@@ -433,6 +478,25 @@
   return BytesToSkip - LengthDiff;
 }
 
+void Preprocessor::SuggestTypoedDirective(const Token ,
+ 

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:1257
   // If we reached here, the preprocessing token is not valid!
-  Diag(Result, diag::err_pp_invalid_directive);
+  Diag(Result, diag::err_pp_invalid_directive) << 0;
 

aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > I think we should be attempting to suggest a typo for the error case as 
> > > well e.g.,
> > > ```
> > > #fi WHATEVER
> > > #endif
> > > ```
> > > we currently give no suggestion for that typo, just the error. However, 
> > > this may require a fair amount of changes because of the various edge 
> > > cases where we give better diagnostics than "unknown directive". e.g.,
> > > ```
> > > #if WHATEVER // error: unterminated conditional directive
> > > #endfi // no diagnostic
> > > ```
> > > so if it looks like covering error cases is going to be involved, I'm 
> > > fine doing it in a follow-up if you'd prefer.
> > The former can be implemented easily, but the latter seems not easy.
> > So what about doing the latter in another patch?
> I'm fine doing the error cases entirely in another patch. The current 
> approach here is a problem because it issues a warning and an error for the 
> same line of code. So I'd go back to the way the code was before, and in a 
> follow-up you can handle errors.
> 
> I think one approach to consider for that is having an argument to 
> `SuggestTypoedDirective()` as to whether to err or warn so that we can reuse 
> the same logic.
OK, reverting :+1:

I'll work on it. Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428985.
ken-matsui added a comment.

Add test for errored directive but no suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c

Index: clang/test/Preprocessor/suggest-typoed-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.c
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=pre-c2x-cpp2b %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=c2x-cpp2b %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=c2x-cpp2b %s
+
+// id:pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   not suggested to '#elifdef'
+// elfindef:  not suggested to '#elifdef'
+// elfinndef: not suggested to '#elifndef'
+// els:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#endif'?}}
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+// id:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifndef'?}}
+// els:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#endif'?}}
+
+#ifdef UNDEFINED
+#i // no diagnostic
+#endif
+
+#if special_compiler
+#special_compiler_directive // no diagnostic
+#endif
+
+// pre-c2x-cpp2b-error@+1 {{invalid preprocessing directive, did you mean '#if'?}}
+#id UNDEFINED
+// c2x-cpp2b-error@-1 {{invalid preprocessing directive, did you mean '#if'?}}
+
+// pre-c2x-cpp2b-error@+1 {{invalid preprocessing directive}}
+#invalid
+// c2x-cpp2b-error@-1 {{invalid preprocessing directive}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -266,6 +266,51 @@
 .Default(false);
 }
 
+/// Find a similar string in `Candidates`.
+///
+/// \param LHS a string for a similar string in `Candidates`
+///
+/// \param Candidates the candidates to find a similar string.
+///
+/// \returns a similar string if exists. If no similar string exists,
+/// returns None.
+static Optional findSimilarStr(
+StringRef LHS, const std::vector ) {
+  // We need to check if `Candidates` has the exact case-insensitive string
+  // because the Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (LHS.equals_insensitive(C)) {
+  return C;
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If the LHS size is less than 3, use the LHS size minus 1 and if not,
+  // use the LHS size divided by 3.
+  size_t Length = LHS.size();
+  size_t MaxDist = Length < 3 ? Length - 1 : Length / 3;
+
+  Optional> SimilarStr = None;
+  for (StringRef C : Candidates) {
+size_t CurDist = LHS.edit_distance(C, true);
+if (CurDist <= MaxDist) {
+  if (!SimilarStr.hasValue()) {
+// The first similar string found.
+SimilarStr = {C, CurDist};
+  } else if (CurDist < SimilarStr->second) {
+// More similar string found.
+SimilarStr = {C, CurDist};
+  }
+}
+  }
+
+  if (SimilarStr.hasValue()) {
+return 

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428983.
ken-matsui added a comment.

Update the code as reviewed and add a release note


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c

Index: clang/test/Preprocessor/suggest-typoed-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.c
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=pre-c2x-cpp2b %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=c2x-cpp2b %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=c2x-cpp2b %s
+
+// id:pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   not suggested to '#elifdef'
+// elfindef:  not suggested to '#elifdef'
+// elfinndef: not suggested to '#elifndef'
+// els:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#endif'?}}
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+// id:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifndef'?}}
+// els:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#endif'?}}
+
+#ifdef UNDEFINED
+#i // no diagnostic
+#endif
+
+#if special_compiler
+#special_compiler_directive // no diagnostic
+#endif
+
+// pre-c2x-cpp2b-error@+1 {{invalid preprocessing directive, did you mean '#if'?}}
+#id UNDEFINED
+// c2x-cpp2b-error@-1 {{invalid preprocessing directive, did you mean '#if'?}}
+// pre-c2x-cpp2b-error@+1 {{#endif without #if}}
+#endif
+// c2x-cpp2b-error@-1 {{#endif without #if}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -266,6 +266,51 @@
 .Default(false);
 }
 
+/// Find a similar string in `Candidates`.
+///
+/// \param LHS a string for a similar string in `Candidates`
+///
+/// \param Candidates the candidates to find a similar string.
+///
+/// \returns a similar string if exists. If no similar string exists,
+/// returns None.
+static Optional findSimilarStr(
+StringRef LHS, const std::vector ) {
+  // We need to check if `Candidates` has the exact case-insensitive string
+  // because the Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (LHS.equals_insensitive(C)) {
+  return C;
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If the LHS size is less than 3, use the LHS size minus 1 and if not,
+  // use the LHS size divided by 3.
+  size_t Length = LHS.size();
+  size_t MaxDist = Length < 3 ? Length - 1 : Length / 3;
+
+  Optional> SimilarStr = None;
+  for (StringRef C : Candidates) {
+size_t CurDist = LHS.edit_distance(C, true);
+if (CurDist <= MaxDist) {
+  if (!SimilarStr.hasValue()) {
+// The first similar string found.
+SimilarStr = {C, CurDist};
+  } else if (CurDist < SimilarStr->second) {
+// More similar string found.
+SimilarStr = {C, CurDist};
+  }
+}
+  }
+
+  if (SimilarStr.hasValue()) {
+return SimilarStr->first;
+  } else {
+

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you for your review :)




Comment at: clang/lib/Lex/PPDirectives.cpp:1257
   // If we reached here, the preprocessing token is not valid!
-  Diag(Result, diag::err_pp_invalid_directive);
+  Diag(Result, diag::err_pp_invalid_directive) << 0;
 

aaron.ballman wrote:
> I think we should be attempting to suggest a typo for the error case as well 
> e.g.,
> ```
> #fi WHATEVER
> #endif
> ```
> we currently give no suggestion for that typo, just the error. However, this 
> may require a fair amount of changes because of the various edge cases where 
> we give better diagnostics than "unknown directive". e.g.,
> ```
> #if WHATEVER // error: unterminated conditional directive
> #endfi // no diagnostic
> ```
> so if it looks like covering error cases is going to be involved, I'm fine 
> doing it in a follow-up if you'd prefer.
The former can be implemented easily, but the latter seems not easy.
So what about doing the latter in another patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

I fixed it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

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

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428910.
ken-matsui added a comment.

Fix the failed test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/Inputs/unsafe-macro-2.h
  clang/test/Lexer/deprecate-macro.c
  clang/test/Preprocessor/elifdef.c
  clang/test/Preprocessor/ext-pp-directive.c
  clang/test/Preprocessor/if_warning.c
  clang/test/Preprocessor/ifdef-recover.c
  clang/test/Preprocessor/macro_misc.c
  clang/test/Preprocessor/macro_vaopt_check.cpp

Index: clang/test/Preprocessor/macro_vaopt_check.cpp
===
--- clang/test/Preprocessor/macro_vaopt_check.cpp
+++ clang/test/Preprocessor/macro_vaopt_check.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++20
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++11
-// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -pedantic -std=c99
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++20
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++11
+// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -Wno-c2x-extensions -pedantic -std=c99
 
 //expected-error@+1{{missing '('}}
 #define V1(...) __VA_OPT__  
Index: clang/test/Preprocessor/macro_misc.c
===
--- clang/test/Preprocessor/macro_misc.c
+++ clang/test/Preprocessor/macro_misc.c
@@ -4,6 +4,7 @@
 #ifdef defined
 #elifdef defined
 #endif
+// expected-warning@-2 {{use of a '#elifdef' directive is a C2x extension}}
 
 
 
Index: clang/test/Preprocessor/ifdef-recover.c
===
--- clang/test/Preprocessor/ifdef-recover.c
+++ clang/test/Preprocessor/ifdef-recover.c
@@ -19,12 +19,14 @@
 #if f(2
 #endif
 
-/* expected-error@+2 {{macro name missing}} */
+/* expected-error@+3 {{macro name missing}} */
+/* expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}} */
 #ifdef FOO
 #elifdef
 #endif
 
-/* expected-error@+2 {{macro name must be an identifier}} */
+/* expected-error@+3 {{macro name must be an identifier}} */
+/* expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}} */
 #ifdef FOO
 #elifdef !
 #endif
Index: clang/test/Preprocessor/if_warning.c
===
--- clang/test/Preprocessor/if_warning.c
+++ clang/test/Preprocessor/if_warning.c
@@ -5,6 +5,7 @@
 #if foo   // expected-error {{'foo' is not defined, evaluates to 0}}
 #endif
 
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}
 #ifdef foo
 #elifdef foo
 #endif
@@ -14,6 +15,7 @@
 
 
 // PR3938
+// expected-warning@+3 {{use of a '#elifdef' directive is a C2x extension}}
 #if 0
 #ifdef D
 #elifdef D
Index: clang/test/Preprocessor/ext-pp-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/ext-pp-directive.c
@@ -0,0 +1,59 @@
+// For C
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=pre-c2x-pedantic -pedantic %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=pre-c2x-compat -Wpre-c2x-compat %s
+// RUN: not %clang_cc1 -std=c99 -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify %s
+
+// For C++
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=pre-cpp2b-pedantic -pedantic %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=pre-cpp2b-compat -Wpre-c++2b-compat %s
+// RUN: not %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify %s
+
+int x;
+
+#if 1
+#elifdef A // #1
+#endif
+// For C
+// pre-c2x-pedantic-warning@#1 {{use of a '#elifdef' directive is a C2x extension}}
+// pre-c2x-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C standards before C2x}}
+
+// For C++
+// pre-cpp2b-pedantic-warning@#1 {{use of a '#elifdef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C++ standards before C++2b}}
+
+#if 1
+#elifndef B // #2
+#endif
+// For C
+// pre-c2x-pedantic-warning@#2 {{use of a '#elifndef' directive is a C2x extension}}
+// pre-c2x-compat-warning@#2 {{use of a '#elifndef' directive is incompatible with C standards before C2x}}
+
+// For C++
+// pre-cpp2b-pedantic-warning@#2 {{use of a '#elifndef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#2 {{use of a '#elifndef' directive is incompatible with C++ standards before C++2b}}
+
+#if 0
+#elifdef C
+#endif
+// For C
+// pre-c2x-pedantic-warning@-3 {{use of a 

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you so much for your review!

My public email address is: `07softy_br...@icloud.com`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

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

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428840.
ken-matsui added a comment.

Updated the code as reviewed, added a release note, and merged 
`ext-cpp2b-pp-directive.cpp` & `ext-c2x-pp-directive.c` into 
`ext-pp-directive.c`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/Inputs/unsafe-macro-2.h
  clang/test/Lexer/deprecate-macro.c
  clang/test/Preprocessor/elifdef.c
  clang/test/Preprocessor/ext-pp-directive.c
  clang/test/Preprocessor/if_warning.c
  clang/test/Preprocessor/ifdef-recover.c
  clang/test/Preprocessor/macro_misc.c
  clang/test/Preprocessor/macro_vaopt_check.cpp

Index: clang/test/Preprocessor/macro_vaopt_check.cpp
===
--- clang/test/Preprocessor/macro_vaopt_check.cpp
+++ clang/test/Preprocessor/macro_vaopt_check.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++20
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++11
-// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -pedantic -std=c99
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++20
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++11
+// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -Wno-c2x-extensions -pedantic -std=c99
 
 //expected-error@+1{{missing '('}}
 #define V1(...) __VA_OPT__  
Index: clang/test/Preprocessor/macro_misc.c
===
--- clang/test/Preprocessor/macro_misc.c
+++ clang/test/Preprocessor/macro_misc.c
@@ -4,6 +4,7 @@
 #ifdef defined
 #elifdef defined
 #endif
+// expected-warning@-2 {{use of a '#elifdef' directive is a C2x extension}}
 
 
 
Index: clang/test/Preprocessor/ifdef-recover.c
===
--- clang/test/Preprocessor/ifdef-recover.c
+++ clang/test/Preprocessor/ifdef-recover.c
@@ -19,12 +19,14 @@
 #if f(2
 #endif
 
-/* expected-error@+2 {{macro name missing}} */
+/* expected-error@+3 {{macro name missing}} */
+/* expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}} */
 #ifdef FOO
 #elifdef
 #endif
 
-/* expected-error@+2 {{macro name must be an identifier}} */
+/* expected-error@+3 {{macro name must be an identifier}} */
+/* expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}} */
 #ifdef FOO
 #elifdef !
 #endif
Index: clang/test/Preprocessor/if_warning.c
===
--- clang/test/Preprocessor/if_warning.c
+++ clang/test/Preprocessor/if_warning.c
@@ -5,6 +5,7 @@
 #if foo   // expected-error {{'foo' is not defined, evaluates to 0}}
 #endif
 
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}
 #ifdef foo
 #elifdef foo
 #endif
@@ -14,6 +15,7 @@
 
 
 // PR3938
+// expected-warning@+3 {{use of a '#elifdef' directive is a C2x extension}}
 #if 0
 #ifdef D
 #elifdef D
Index: clang/test/Preprocessor/ext-pp-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/ext-pp-directive.c
@@ -0,0 +1,59 @@
+// For C
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=pre-c2x-pedantic -pedantic %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=pre-c2x-compat -Wpre-c2x-compat %s
+// RUN: not %clang_cc1 -std=c99 -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify %s
+
+// For C++
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=pre-cpp2b-pedantic -pedantic %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=pre-cpp2b-compat -Wpre-c++2b-compat %s
+// RUN: not %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify %s
+
+int x;
+
+#if 1
+#elifdef A // #1
+#endif
+// For C
+// pre-c2x-pedantic-warning@#1 {{use of a '#elifdef' directive is a C2x extension}}
+// pre-c2x-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C standards before C2x}}
+
+// For C++
+// pre-cpp2b-pedantic-warning@#1 {{use of a '#elifdef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C++ standards before C++2b}}
+
+#if 1
+#elifndef B // #2
+#endif
+// For C
+// pre-c2x-pedantic-warning@#2 {{use of a '#elifndef' directive is a C2x extension}}
+// pre-c2x-compat-warning@#2 {{use of a '#elifndef' directive is incompatible with C standards before C2x}}
+
+// For C++
+// pre-cpp2b-pedantic-warning@#2 {{use of a '#elifndef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#2 {{use of a '#elifndef' directive is 

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/test/Preprocessor/suggest-typoed-directive.c:36
+// elfidef:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you 
mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you 
mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you 
mean '#elifndef'?}}

Here, `#elfindef` is suggested to `#elifdef`, not `#elifndef`, but I believe it 
will help many users to realize that they have typo'd `#elifndef`, or maybe 
they might have meant actually `#elifdef`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428827.
ken-matsui added a comment.

Remove unused includes in `StringRef.h`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c

Index: clang/test/Preprocessor/suggest-typoed-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.c
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=not-c2x-cpp2b %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=c2x-cpp2b %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=c2x-cpp2b %s
+
+// id:not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   not suggested to '#elifdef'
+// elfindef:  not suggested to '#elifdef'
+// elfinndef: not suggested to '#elifndef'
+// els:   not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#endif'?}}
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+// id:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifndef'?}}
+// els:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#endif'?}}
+
+#ifdef UNDEFINED
+#i // no diagnostic
+#endif
+
+#if special_compiler
+#special_compiler_directive // no diagnostic
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -266,6 +266,51 @@
 .Default(false);
 }
 
+/// Find a similar string in `Candidates`.
+///
+/// \param LHS a string for a similar string in `Candidates`
+///
+/// \param Candidates the candidates to find a similar string.
+///
+/// \returns a similar string if exists. If no similar string exists,
+/// returns None.
+static Optional findSimilarStr(
+StringRef LHS, const std::vector ) {
+  // We need to check if `Candidates` has the exact case-insensitive string
+  // because the Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (LHS.equals_insensitive(C)) {
+  return C;
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If the LHS size is less than 3, use the LHS size minus 1 and if not,
+  // use the LHS size divided by 3.
+  size_t Length = LHS.size();
+  size_t MaxDist = Length < 3 ? Length - 1 : Length / 3;
+
+  Optional> SimilarStr = None;
+  for (StringRef C : Candidates) {
+size_t CurDist = LHS.edit_distance(C, true);
+if (CurDist <= MaxDist) {
+  if (!SimilarStr.hasValue()) {
+// The first similar string found.
+SimilarStr = {C, CurDist};
+  } else if (CurDist < SimilarStr->second) {
+// More similar string found.
+SimilarStr = {C, CurDist};
+  }
+}
+  }
+
+  if (SimilarStr.hasValue()) {
+return SimilarStr->first;
+  } else {
+return None;
+  }
+}
+
 bool Preprocessor::CheckMacroName(Token , MacroUse isDefineUndef,
   bool *ShadowFlag) {
   // Missing macro name?
@@ -433,6 +478,27 @@
   return BytesToSkip - LengthDiff;
 }
 
+/// SuggestTypoedDirective - Provide a suggestion for a typoed directive. If
+/// there is no typo, 

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you for your support!

I updated the code, so could you please review this patch again?




Comment at: llvm/lib/Support/StringRef.cpp:102
+// Find a similar string in `Candidates`.
+Optional StringRef::find_similar_str(const std::vector 
, size_t Dist) const {
+  // We need to check if `rng` has the exact case-insensitive string because 
the Levenshtein distance match does not

aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > I am less convinced that we want this as a general interface on 
> > > `StringRef`. I think callers need to decide whether something is similar 
> > > enough or not. For example, case sensitivity may not matter for this case 
> > > but it might matter to another case. Others may wish to limit the max 
> > > edit_distance for each of the candidates for performance reasons, others 
> > > may not. We already saw there were questions about whether to allow 
> > > replacements or not. That sort of thing.
> > > 
> > > I think this functionality should be moved to PPDirectives.cpp as a 
> > > static helper function that's specific to this use. Also, because this 
> > > returns one of the candidates passed in, and those are a `StringRef`, I 
> > > think this function should return a `StringRef` -- this removes 
> > > allocation costs. I'd also drop the `Dist` parameter as the function is 
> > > no longer intended to be a general purpose one.
> > I am also in favor of it, but where should I put tests for this 
> > functionality?
> > Is it possible to write unit tests for static functions implemented in 
> > `.cpp` files?
> This is something we wouldn't usually add tests for directly but instead test 
> the behavior of through lit -- the tests you already have exercise this code 
> path and would show if there's a situation where we expect a viable candidate 
> and don't find any. So I'd not worry about test coverage for it in this case.
I see. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428826.
ken-matsui added a comment.

Update the code as reviewed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c
  llvm/include/llvm/ADT/StringRef.h

Index: llvm/include/llvm/ADT/StringRef.h
===
--- llvm/include/llvm/ADT/StringRef.h
+++ llvm/include/llvm/ADT/StringRef.h
@@ -10,6 +10,7 @@
 #define LLVM_ADT_STRINGREF_H
 
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Compiler.h"
@@ -24,6 +25,7 @@
 #endif
 #include 
 #include 
+#include 
 
 // Declare the __builtin_strlen intrinsic for MSVC so it can be used in
 // constexpr context.
Index: clang/test/Preprocessor/suggest-typoed-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.c
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=not-c2x-cpp2b %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=c2x-cpp2b %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=c2x-cpp2b %s
+
+// id:not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   not suggested to '#elifdef'
+// elfindef:  not suggested to '#elifdef'
+// elfinndef: not suggested to '#elifndef'
+// els:   not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#endif'?}}
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+// id:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifndef'?}}
+// els:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#endif'?}}
+
+#ifdef UNDEFINED
+#i // no diagnostic
+#endif
+
+#if special_compiler
+#special_compiler_directive // no diagnostic
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -266,6 +266,51 @@
 .Default(false);
 }
 
+/// Find a similar string in `Candidates`.
+///
+/// \param LHS a string for a similar string in `Candidates`
+///
+/// \param Candidates the candidates to find a similar string.
+///
+/// \returns a similar string if exists. If no similar string exists,
+/// returns None.
+static Optional findSimilarStr(
+StringRef LHS, const std::vector ) {
+  // We need to check if `Candidates` has the exact case-insensitive string
+  // because the Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (LHS.equals_insensitive(C)) {
+  return C;
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If the LHS size is less than 3, use the LHS size minus 1 and if not,
+  // use the LHS size divided by 3.
+  size_t Length = LHS.size();
+  size_t MaxDist = Length < 3 ? Length - 1 : Length / 3;
+
+  Optional> SimilarStr = None;
+  for (StringRef C : Candidates) {
+size_t CurDist = LHS.edit_distance(C, true);
+if (CurDist <= MaxDist) {
+  if (!SimilarStr.hasValue()) {
+// The first similar string 

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/test/Preprocessor/suggest-typoed-directive.c:10
+// expected-warning@+11 {{'#elfidef' directive not found, did you mean 
'#elifdef'?}}
+// expected-warning@+11 {{'#elfindef' directive not found, did you mean 
'#elifdef'?}}
+// expected-warning@+11 {{'#elsi' directive not found, did you mean '#else'?}}

aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > erichkeane wrote:
> > > > aaron.ballman wrote:
> > > > > ken-matsui wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > ken-matsui wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > ken-matsui wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > It's interesting that this one suggested `#elifdef` 
> > > > > > > > > > > instead of `#elifndef` -- is there anything that can be 
> > > > > > > > > > > done for that?
> > > > > > > > > > > 
> > > > > > > > > > > Also, one somewhat interesting question is whether we 
> > > > > > > > > > > want to recommend `#elifdef` and `#elifndef` outside of 
> > > > > > > > > > > C2x/C++2b mode. Those directives only exist in the latest 
> > > > > > > > > > > language standard, but Clang supports them as a 
> > > > > > > > > > > conforming extension in all language modes. Given that 
> > > > > > > > > > > this diagnostic is about typos, I think I'm okay 
> > > > > > > > > > > suggesting the directives even in older language modes. 
> > > > > > > > > > > That's as likely to be a correct suggestion as not, IMO.
> > > > > > > > > > > It's interesting that this one suggested `#elifdef` 
> > > > > > > > > > > instead of `#elifndef` -- is there anything that can be 
> > > > > > > > > > > done for that?
> > > > > > > > > > 
> > > > > > > > > > I found I have to use `std::min_element` instead of 
> > > > > > > > > > `std::max_element` because we are finding the nearest (most 
> > > > > > > > > > minimum distance) string. Meanwhile, `#elfindef` has 2 
> > > > > > > > > > distance with both `#elifdef` and `#elifndef`. After 
> > > > > > > > > > replacing `std::max_element` with `std::min_element`, I 
> > > > > > > > > > could suggest `#elifndef` from `#elfinndef`.
> > > > > > > > > > 
> > > > > > > > > > > Also, one somewhat interesting question is whether we 
> > > > > > > > > > > want to recommend `#elifdef` and `#elifndef` outside of 
> > > > > > > > > > > C2x/C++2b mode. Those directives only exist in the latest 
> > > > > > > > > > > language standard, but Clang supports them as a 
> > > > > > > > > > > conforming extension in all language modes. Given that 
> > > > > > > > > > > this diagnostic is about typos, I think I'm okay 
> > > > > > > > > > > suggesting the directives even in older language modes. 
> > > > > > > > > > > That's as likely to be a correct suggestion as not, IMO.
> > > > > > > > > > 
> > > > > > > > > > I agree with you because Clang implements those directives, 
> > > > > > > > > > and the suggested code will also be compilable. I 
> > > > > > > > > > personally think only not compilable suggestions should be 
> > > > > > > > > > avoided. (Or, we might place additional info for outside of 
> > > > > > > > > > C2x/C++2b mode like `this is a C2x/C++2b feature but 
> > > > > > > > > > compilable on Clang`?)
> > > > > > > > > > 
> > > > > > > > > > ---
> > > > > > > > > > 
> > > > > > > > > > According to the algorithm of `std::min_element`, we only 
> > > > > > > > > > get an iterator of the first element even if there is 
> > > > > > > > > > another element that has the same distance. So, `#elfindef` 
> > > > > > > > > > only suggests `#elifdef` in accordance with the order of 
> > > > > > > > > > `Candidates`, and I don't think it is beautiful to depend 
> > > > > > > > > > on the order of candidates. I would say that we can suggest 
> > > > > > > > > > all the same distance like the following, but I'm not sure 
> > > > > > > > > > this is preferable:
> > > > > > > > > > 
> > > > > > > > > > ```
> > > > > > > > > > #elfindef // diag: unknown directive, did you mean #elifdef 
> > > > > > > > > > or #elifndef?
> > > > > > > > > > ```
> > > > > > > > > > 
> > > > > > > > > > I agree with you because Clang implements those directives, 
> > > > > > > > > > and the suggested code will also be compilable. I 
> > > > > > > > > > personally think only not compilable suggestions should be 
> > > > > > > > > > avoided. (Or, we might place additional info for outside of 
> > > > > > > > > > C2x/C++2b mode like this is a C2x/C++2b feature but 
> > > > > > > > > > compilable on Clang?)
> > > > > > > > > 
> > > > > > > > > I may be changing my mind on this a bit. I now see we don't 
> > > > > > > > > issue an extension warning when using `#elifdef` or 
> > > > > > > > > `#elifndef` in older language modes. That means suggesting 
> > > > > > > > > those will be correct but only for Clang, and the user won't 
> > > > > > > > > have any other diagnostics to tell them about the portability 
> > > 

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Hello @Jake-Egan,

I believe the problem was already fixed by the @aaron.ballman's patch here 
.

Thank you for fixing it, @aaron.ballman!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you for your review! I updated the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

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

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428636.
ken-matsui added a comment.

Update the code as reviewed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/Inputs/unsafe-macro-2.h
  clang/test/Lexer/deprecate-macro.c
  clang/test/Preprocessor/elifdef.c
  clang/test/Preprocessor/ext-c2x-pp-directive.c
  clang/test/Preprocessor/ext-cpp2b-pp-directive.cpp
  clang/test/Preprocessor/if_warning.c
  clang/test/Preprocessor/ifdef-recover.c
  clang/test/Preprocessor/macro_misc.c
  clang/test/Preprocessor/macro_vaopt_check.cpp

Index: clang/test/Preprocessor/macro_vaopt_check.cpp
===
--- clang/test/Preprocessor/macro_vaopt_check.cpp
+++ clang/test/Preprocessor/macro_vaopt_check.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++20
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++11
-// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -pedantic -std=c99
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++20
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++11
+// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -Wno-c2x-extensions -pedantic -std=c99
 
 //expected-error@+1{{missing '('}}
 #define V1(...) __VA_OPT__  
Index: clang/test/Preprocessor/macro_misc.c
===
--- clang/test/Preprocessor/macro_misc.c
+++ clang/test/Preprocessor/macro_misc.c
@@ -4,6 +4,7 @@
 #ifdef defined
 #elifdef defined
 #endif
+// expected-warning@-2 {{use of a '#elifdef' directive is a C2x extension}}
 
 
 
Index: clang/test/Preprocessor/ifdef-recover.c
===
--- clang/test/Preprocessor/ifdef-recover.c
+++ clang/test/Preprocessor/ifdef-recover.c
@@ -19,12 +19,14 @@
 #if f(2
 #endif
 
-/* expected-error@+2 {{macro name missing}} */
+/* expected-error@+3 {{macro name missing}} */
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}
 #ifdef FOO
 #elifdef
 #endif
 
-/* expected-error@+2 {{macro name must be an identifier}} */
+/* expected-error@+3 {{macro name must be an identifier}} */
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}
 #ifdef FOO
 #elifdef !
 #endif
Index: clang/test/Preprocessor/if_warning.c
===
--- clang/test/Preprocessor/if_warning.c
+++ clang/test/Preprocessor/if_warning.c
@@ -5,6 +5,7 @@
 #if foo   // expected-error {{'foo' is not defined, evaluates to 0}}
 #endif
 
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}
 #ifdef foo
 #elifdef foo
 #endif
@@ -14,6 +15,7 @@
 
 
 // PR3938
+// expected-warning@+3 {{use of a '#elifdef' directive is a C2x extension}}
 #if 0
 #ifdef D
 #elifdef D
Index: clang/test/Preprocessor/ext-cpp2b-pp-directive.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/ext-cpp2b-pp-directive.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=pre-cpp2b-pedantic -pedantic %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=pre-cpp2b-compat -Wpre-c++2b-compat %s
+// RUN: not %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify %s
+
+int x;
+
+#if 1
+#elifdef A // #1
+#endif
+// pre-cpp2b-pedantic-warning@#1 {{use of a '#elifdef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C++ standards before C++2b}}
+
+#if 1
+#elifndef B // #2
+#endif
+// pre-cpp2b-pedantic-warning@#2 {{use of a '#elifndef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#2 {{use of a '#elifndef' directive is incompatible with C++ standards before C++2b}}
+
+#if 0
+#elifdef C
+#endif
+// pre-cpp2b-pedantic-warning@-2 {{use of a '#elifdef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@-3 {{use of a '#elifdef' directive is incompatible with C++ standards before C++2b}}
+
+#if 0
+#elifndef D
+#endif
+// pre-cpp2b-pedantic-warning@-2 {{use of a '#elifndef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@-3 {{use of a '#elifndef' directive is incompatible with C++ standards before C++2b}}
Index: clang/test/Preprocessor/ext-c2x-pp-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/ext-c2x-pp-directive.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=c99-pedantic -pedantic %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=pre-c2x-compat -Wpre-c2x-compat %s
+// RUN: not 

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.



> That looks to be exactly what I needed, but you can double-check that you 
> were properly attributed here: 
> https://github.com/llvm/llvm-project/commit/786c721c2bbd2e0646e314671e010859550423bf
> I added one when I landed. I also adjusted the commit message slightly from 
> what you have here in Phab for some added clarity.

Thank you! I was able to confirm that I was attributed.

In D124534#3505729 , @aaron.ballman 
wrote:

> In D124534#3505709 , @aaron.ballman 
> wrote:
>
>> In D124534#3504641 , @ken-matsui 
>> wrote:
>>
>>> (I use this info for every commit on GitHub, but is this what you expected? 
>>> Please let me know if you need an email where you can be reached out.)
>>
>> That looks to be exactly what I needed, but you can double-check that you 
>> were properly attributed here: 
>> https://github.com/llvm/llvm-project/commit/786c721c2bbd2e0646e314671e010859550423bf
>
> Actually, it does raise an interesting issue -- I don't think you'll get 
> build failure emails when using that address. After a patch is landed, a 
> bunch of build bots test the patch out and when there's a failure, they send 
> email to everyone on the blame list. You should probably use an email address 
> that receives email for future commits (or when you go to obtain your own 
> commit privileges) so that you can receive those emails.

Ah, sorry. I also have a public email for GitHub (just mostly used the private 
one), so I'll share a public one in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

In D124534#3504610 , @aaron.ballman 
wrote:

> LGTM, thank you for all the hard work on this! I assume you need me to land 
> this on your behalf -- if so, can you let me know what name and email address 
> you would like me to use for patch attribution?

Thank you so much for your support and approval!

Yes, I do not have commit access; could you please use the following info for 
this patch?

Name: Ken Matsui
Email: 26405363+ken-mat...@users.noreply.github.com

(I use this info for every commit on GitHub, but is this what you expected? 
Please let me know if you need an email where you can be reached out.)

> I think we should add a release note for this to let folks know about the new 
> warning group (there's a "Improvements to Clang's diagnostics" in 
> clang/docs/ReleaseNotes.rst). You can either add one to this patch, or I can 
> add one for you when I go to land, I'm fine either way.

Could you please add it to this patch for this time?
I would like to add one next time by referring to how you do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/test/Preprocessor/suggest-typoed-directive.c:10
+// expected-warning@+11 {{'#elfidef' directive not found, did you mean 
'#elifdef'?}}
+// expected-warning@+11 {{'#elfindef' directive not found, did you mean 
'#elifdef'?}}
+// expected-warning@+11 {{'#elsi' directive not found, did you mean '#else'?}}

aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > ken-matsui wrote:
> > > > aaron.ballman wrote:
> > > > > ken-matsui wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > ken-matsui wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > It's interesting that this one suggested `#elifdef` instead 
> > > > > > > > > of `#elifndef` -- is there anything that can be done for that?
> > > > > > > > > 
> > > > > > > > > Also, one somewhat interesting question is whether we want to 
> > > > > > > > > recommend `#elifdef` and `#elifndef` outside of C2x/C++2b 
> > > > > > > > > mode. Those directives only exist in the latest language 
> > > > > > > > > standard, but Clang supports them as a conforming extension 
> > > > > > > > > in all language modes. Given that this diagnostic is about 
> > > > > > > > > typos, I think I'm okay suggesting the directives even in 
> > > > > > > > > older language modes. That's as likely to be a correct 
> > > > > > > > > suggestion as not, IMO.
> > > > > > > > > It's interesting that this one suggested `#elifdef` instead 
> > > > > > > > > of `#elifndef` -- is there anything that can be done for that?
> > > > > > > > 
> > > > > > > > I found I have to use `std::min_element` instead of 
> > > > > > > > `std::max_element` because we are finding the nearest (most 
> > > > > > > > minimum distance) string. Meanwhile, `#elfindef` has 2 distance 
> > > > > > > > with both `#elifdef` and `#elifndef`. After replacing 
> > > > > > > > `std::max_element` with `std::min_element`, I could suggest 
> > > > > > > > `#elifndef` from `#elfinndef`.
> > > > > > > > 
> > > > > > > > > Also, one somewhat interesting question is whether we want to 
> > > > > > > > > recommend `#elifdef` and `#elifndef` outside of C2x/C++2b 
> > > > > > > > > mode. Those directives only exist in the latest language 
> > > > > > > > > standard, but Clang supports them as a conforming extension 
> > > > > > > > > in all language modes. Given that this diagnostic is about 
> > > > > > > > > typos, I think I'm okay suggesting the directives even in 
> > > > > > > > > older language modes. That's as likely to be a correct 
> > > > > > > > > suggestion as not, IMO.
> > > > > > > > 
> > > > > > > > I agree with you because Clang implements those directives, and 
> > > > > > > > the suggested code will also be compilable. I personally think 
> > > > > > > > only not compilable suggestions should be avoided. (Or, we 
> > > > > > > > might place additional info for outside of C2x/C++2b mode like 
> > > > > > > > `this is a C2x/C++2b feature but compilable on Clang`?)
> > > > > > > > 
> > > > > > > > ---
> > > > > > > > 
> > > > > > > > According to the algorithm of `std::min_element`, we only get 
> > > > > > > > an iterator of the first element even if there is another 
> > > > > > > > element that has the same distance. So, `#elfindef` only 
> > > > > > > > suggests `#elifdef` in accordance with the order of 
> > > > > > > > `Candidates`, and I don't think it is beautiful to depend on 
> > > > > > > > the order of candidates. I would say that we can suggest all 
> > > > > > > > the same distance like the following, but I'm not sure this is 
> > > > > > > > preferable:
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > #elfindef // diag: unknown directive, did you mean #elifdef or 
> > > > > > > > #elifndef?
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > I agree with you because Clang implements those directives, and 
> > > > > > > > the suggested code will also be compilable. I personally think 
> > > > > > > > only not compilable suggestions should be avoided. (Or, we 
> > > > > > > > might place additional info for outside of C2x/C++2b mode like 
> > > > > > > > this is a C2x/C++2b feature but compilable on Clang?)
> > > > > > > 
> > > > > > > I may be changing my mind on this a bit. I now see we don't issue 
> > > > > > > an extension warning when using `#elifdef` or `#elifndef` in 
> > > > > > > older language modes. That means suggesting those will be correct 
> > > > > > > but only for Clang, and the user won't have any other diagnostics 
> > > > > > > to tell them about the portability issue. And those particular 
> > > > > > > macros are reasonably likely to be used in a header where the 
> > > > > > > user is trying to aim for portability. So I'm starting to think 
> > > > > > > we should only suggest those two in C2x mode (and we should 
> > > > > > > probably add a portability warning for #elifdef and #elifndef, so 
> > > > > > > I filed: https://github.com/llvm/llvm-project/issues/55306)
> > > > 

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428471.
ken-matsui added a comment.

Set `AllowReplacements` to `true`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c
  llvm/include/llvm/ADT/StringRef.h
  llvm/lib/Support/StringRef.cpp
  llvm/unittests/ADT/StringRefTest.cpp

Index: llvm/unittests/ADT/StringRefTest.cpp
===
--- llvm/unittests/ADT/StringRefTest.cpp
+++ llvm/unittests/ADT/StringRefTest.cpp
@@ -566,6 +566,13 @@
 }
 
 TEST(StringRefTest, EditDistance) {
+  EXPECT_EQ(0U, StringRef("").edit_distance(""));
+  EXPECT_EQ(1U, StringRef("").edit_distance("aaab"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("aacb"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("abca"));
+  EXPECT_EQ(3U, StringRef("aabc").edit_distance("adef"));
+  EXPECT_EQ(4U, StringRef("abcd").edit_distance("efgh"));
+
   StringRef Hello("hello");
   EXPECT_EQ(2U, Hello.edit_distance("hill"));
 
@@ -584,6 +591,40 @@
"people soiled our green "));
 }
 
+TEST(StringRefTest, FindSimilarStr) {
+  {
+std::vector Candidates{"aaab", "aaac"};
+EXPECT_EQ(std::string("aaab"), StringRef("").find_similar_str(Candidates));
+  }
+  {
+std::vector Candidates{"aab", "aac"};
+EXPECT_EQ(std::string("aab"), StringRef("aaa").find_similar_str(Candidates));
+  }
+  {
+std::vector Candidates{"ab", "ac"};
+EXPECT_EQ(std::string("ab"), StringRef("aa").find_similar_str(Candidates));
+  }
+  {
+std::vector Candidates{"b", "c"};
+EXPECT_EQ(None, StringRef("a").find_similar_str(Candidates));
+  }
+  { // macros
+std::vector Candidates{
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elfidef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(None, StringRef("special_compiler_directive").find_similar_str(Candidates));
+  }
+  { // case-insensitive
+std::vector Candidates{
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("ELIFDEF").find_similar_str(Candidates));
+  }
+}
+
 TEST(StringRefTest, Misc) {
   std::string Storage;
   raw_string_ostream OS(Storage);
Index: llvm/lib/Support/StringRef.cpp
===
--- llvm/lib/Support/StringRef.cpp
+++ llvm/lib/Support/StringRef.cpp
@@ -98,6 +98,43 @@
   AllowReplacements, MaxEditDistance);
 }
 
+// Find a similar string in `Candidates`.
+Optional StringRef::find_similar_str(const std::vector , size_t Dist) const {
+  // We need to check if `rng` has the exact case-insensitive string because the Levenshtein distance match does not
+  // care about it.
+  for (StringRef C : Candidates) {
+if (equals_insensitive(C)) {
+  return C.str();
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If dist is given, use the dist for maxDist; otherwise, if the LHS size is less than 3, use the LHS size minus 1
+  // and if not, use the LHS size divided by 3.
+  size_t MaxDist = Dist != 0? Dist
+   : Length < 3 ? Length - 1
+: Length / 3;
+
+  std::vector> Cand;
+  for (StringRef C : Candidates) {
+size_t CurDist = edit_distance(C, true);
+if (CurDist <= MaxDist) {
+  Cand.emplace_back(C, CurDist);
+}
+  }
+
+  if (Cand.empty()) {
+return None;
+  } else if (Cand.size() == 1) {
+return Cand[0].first;
+  } else {
+auto SimilarStr = std::min_element(
+Cand.cbegin(), Cand.cend(),
+[](const auto , const auto ) { return A.second < B.second; });
+return SimilarStr->first;
+  }
+}
+
 //===--===//
 // String Operations
 //===--===//
Index: llvm/include/llvm/ADT/StringRef.h
===
--- llvm/include/llvm/ADT/StringRef.h
+++ llvm/include/llvm/ADT/StringRef.h
@@ -10,6 +10,7 @@
 #define LLVM_ADT_STRINGREF_H
 
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Compiler.h"
@@ -24,6 +25,7 @@
 #endif
 #include 
 #include 
+#include 
 
 // Declare the __builtin_strlen intrinsic for MSVC so it can be used in
 // constexpr context.
@@ -240,6 +242,23 @@
 unsigned 

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428450.
ken-matsui added a comment.

Fix the test as reviewed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/line-directive-system-headers.c
  clang/test/Preprocessor/line-directive.c
  clang/test/SemaCXX/constexpr-string.cpp

Index: clang/test/SemaCXX/constexpr-string.cpp
===
--- clang/test/SemaCXX/constexpr-string.cpp
+++ clang/test/SemaCXX/constexpr-string.cpp
@@ -7,7 +7,7 @@
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 
-# 9 "/usr/include/string.h" 1 3 4
+# 9 "/usr/include/string.h" 1 3 4  // expected-warning {{this style of line directive is a GNU extension}}
 extern "C" {
   typedef decltype(sizeof(int)) size_t;
 
@@ -29,7 +29,7 @@
 }
 # 25 "SemaCXX/constexpr-string.cpp" 2
 
-# 27 "/usr/include/wchar.h" 1 3 4
+# 27 "/usr/include/wchar.h" 1 3 4  // expected-warning {{this style of line directive is a GNU extension}}
 extern "C" {
 #if NO_PREDEFINED_WCHAR_T
   typedef decltype(L'0') wchar_t;
Index: clang/test/Preprocessor/line-directive.c
===
--- clang/test/Preprocessor/line-directive.c
+++ clang/test/Preprocessor/line-directive.c
@@ -9,8 +9,8 @@
 # 20 "" 2
 
 // a push/pop before any other line control
-# 10 "enter-0" 1
-# 11 "" 2 // pop to main file
+# 10 "enter-0" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 11 "" 2 // pop to main file: expected-warning {{this style of line directive is a GNU extension}}
 #error MAIN7
 // expected-error@-1{{MAIN7}}
 
@@ -27,13 +27,15 @@
 #define A 42 "foo"
 #line A
 
-# 42
-# 42 "foo"
+# 42 // expected-warning {{this style of line directive is a GNU extension}}
+# 42 "foo" // expected-warning {{this style of line directive is a GNU extension}}
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
+// The next two lines do not get diagnosed because they are considered to be
+// within the system header, where diagnostics are suppressed.
 # 42 "foo" 1 3  // enter
 # 42 "foo" 2 3  // exit
 # 42 "foo" 2 3 4 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
-# 42 "foo" 3 4
+# 42 "foo" 3 4 // expected-warning {{this style of line directive is a GNU extension}}
 
 # 'a'// expected-error {{invalid preprocessing directive}}
 # 42 'f' // expected-error {{invalid filename for line marker directive}}
@@ -54,7 +56,7 @@
 
 // Verify that linemarker diddling of the system header flag works.
 
-# 192 "glomp.h" // not a system header.
+# 192 "glomp.h" // not a system header.: expected-warning {{this style of line directive is a GNU extension}}
 typedef int x;  // expected-note {{previous definition is here}}
 typedef int x;  // expected-warning {{redefinition of typedef 'x' is a C11 feature}}
 
@@ -97,7 +99,7 @@
 #line 010  // expected-warning {{#line directive interprets number as decimal, not octal}}
 extern int array[__LINE__ == 10 ? 1:-1];
 
-# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}}
+# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}} expected-warning {{this style of line directive is a GNU extension}}
 extern int array_gnuline[__LINE__ == 20 ? 1:-1];
 
 /* PR3917 */
@@ -106,7 +108,7 @@
 _\
 _LINE__ == 42 ? 1: -1];  /* line marker is location of first _ */
 
-# 51
+# 51 // expected-warning {{this style of line directive is a GNU extension}}
 extern char array2_gnuline[\
 _\
 _LINE__ == 52 ? 1: -1];  /* line marker is location of first _ */
@@ -115,12 +117,12 @@
 #line 0 "line-directive.c" // expected-warning {{#line directive with zero argument is a GNU extension}}
 undefined t; // expected-error {{unknown type name 'undefined'}}
 
-# 115 "main"
-# 116 "enter-1" 1
-# 117 "enter-2" 1
-# 118 "" 2 // pop to enter-1
+# 115 "main" // expected-warning {{this style of line directive is a GNU extension}}
+# 116 "enter-1" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 117 "enter-2" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 118 "" 2 // pop to enter-1: expected-warning {{this style of line directive is a GNU extension}}
 #error ENTER1
 // expected-error@-1{{ENTER1}}
-# 121 "" 2 // pop to "main"
+# 121 "" 2 // pop to "main": expected-warning {{this style of line directive is a GNU extension}}
 #error MAIN2
 // expected-error@-1{{MAIN2}}

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you for your review! I'm going to fix them :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D125078: Implement a feature to show line numbers in diagnostics

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you all so much :)))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125078

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

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428435.
ken-matsui added a comment.

Fix failed tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/Inputs/unsafe-macro-2.h
  clang/test/Lexer/deprecate-macro.c
  clang/test/Preprocessor/elifdef.c
  clang/test/Preprocessor/ext-c2x-pp-directive.c
  clang/test/Preprocessor/ext-cpp2b-pp-directive.cpp
  clang/test/Preprocessor/if_warning.c
  clang/test/Preprocessor/ifdef-recover.c
  clang/test/Preprocessor/macro_misc.c
  clang/test/Preprocessor/macro_vaopt_check.cpp

Index: clang/test/Preprocessor/macro_vaopt_check.cpp
===
--- clang/test/Preprocessor/macro_vaopt_check.cpp
+++ clang/test/Preprocessor/macro_vaopt_check.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++20
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++11
-// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -pedantic -std=c99
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++20
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++11
+// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -Wno-c2x-extensions -pedantic -std=c99
 
 //expected-error@+1{{missing '('}}
 #define V1(...) __VA_OPT__  
Index: clang/test/Preprocessor/macro_misc.c
===
--- clang/test/Preprocessor/macro_misc.c
+++ clang/test/Preprocessor/macro_misc.c
@@ -4,6 +4,7 @@
 #ifdef defined
 #elifdef defined
 #endif
+// expected-warning@-2 {{use of a '#elifdef' directive is a C2x extension}}
 
 
 
Index: clang/test/Preprocessor/ifdef-recover.c
===
--- clang/test/Preprocessor/ifdef-recover.c
+++ clang/test/Preprocessor/ifdef-recover.c
@@ -19,12 +19,14 @@
 #if f(2
 #endif
 
-/* expected-error@+2 {{macro name missing}} */
+/* expected-error@+3 {{macro name missing}} */
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}
 #ifdef FOO
 #elifdef
 #endif
 
-/* expected-error@+2 {{macro name must be an identifier}} */
+/* expected-error@+3 {{macro name must be an identifier}} */
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}
 #ifdef FOO
 #elifdef !
 #endif
Index: clang/test/Preprocessor/if_warning.c
===
--- clang/test/Preprocessor/if_warning.c
+++ clang/test/Preprocessor/if_warning.c
@@ -5,6 +5,7 @@
 #if foo   // expected-error {{'foo' is not defined, evaluates to 0}}
 #endif
 
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}
 #ifdef foo
 #elifdef foo
 #endif
@@ -14,6 +15,7 @@
 
 
 // PR3938
+// expected-warning@+3 {{use of a '#elifdef' directive is a C2x extension}}
 #if 0
 #ifdef D
 #elifdef D
Index: clang/test/Preprocessor/ext-cpp2b-pp-directive.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/ext-cpp2b-pp-directive.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=pre-cpp2b-pedantic -pedantic %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=pre-cpp2b-compat -Wpre-c++2b-compat %s
+// RUN: not %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify %s
+
+int x;
+
+#if 1
+#elifdef A // #1
+#endif
+// pre-cpp2b-pedantic-warning@#1 {{use of a '#elifdef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C++ standards before C++2b}}
+
+#if 1
+#elifndef B // #2
+#endif
+// pre-cpp2b-pedantic-warning@#2 {{use of a '#elifndef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#2 {{use of a '#elifndef' directive is incompatible with C++ standards before C++2b}}
+
+#if 0
+#elifdef C
+#endif
+// pre-cpp2b-pedantic-warning@-2 {{use of a '#elifdef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@-3 {{use of a '#elifdef' directive is incompatible with C++ standards before C++2b}}
+
+#if 0
+#elifndef D
+#endif
+// pre-cpp2b-pedantic-warning@-2 {{use of a '#elifndef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@-3 {{use of a '#elifndef' directive is incompatible with C++ standards before C++2b}}
Index: clang/test/Preprocessor/ext-c2x-pp-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/ext-c2x-pp-directive.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=c99-pedantic -pedantic %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=pre-c2x-compat -Wpre-c2x-compat %s
+// RUN: not %clang_cc1 

[PATCH] D125078: Implement a feature to show line numbers in diagnostics

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125078

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

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:696-698
+def ext_c2x_pp_directive : Extension<
+  "%select{#elifdef|#elifndef}0 is a C2x extension">,
+  InGroup;

aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > ken-matsui wrote:
> > > > aaron.ballman wrote:
> > > > > I think we need two diagnostics, one for C2x and one for C++2b 
> > > > > (https://wg21.link/p2334 was adopted for C++23). Each of these 
> > > > > diagnostics should come in a pair:
> > > > > ```
> > > > > def warn_cxx20_compat_pp_directive : Warning<
> > > > >   "use of a '#%select{elifdef|elifndef}0' directive is incompatible 
> > > > > with C++ standards before C++2b",
> > > > >   InGroup, DefaultIgnore;
> > > > > def ext_cxx20_pp_directive : ExtWarn<
> > > > >   "use of a '#%select{elifdef|elifndef}0' directive is a C++2b 
> > > > > extension",
> > > > >   InGroup;
> > > > > ```
> > > > > and similar for C, except with wording about C standards and in the C 
> > > > > warning groups.
> > > > I thought I had to use `Extension` here, but what is the difference 
> > > > between `Warning`, `ExtWarn`, and `Extension`?
> > > Great question!
> > > 
> > > `Extension` diagnostics are warnings that are enabled via `-pedantic` but 
> > > otherwise off by default.
> > > `ExtWarn` diagnostics are warnings that are enabled via `-pedantic` but 
> > > are also on by default.
> > > `Warning` diagnostics are warnings. You can use things like 
> > > `DefaultIgnore` or `DefaultError` to control the default behavior of the 
> > > warning.
> > > `Error` diagnostics are errors.
> > > 
> > > You'll typically use `Extension` or `ExtWarn` for things that are 
> > > extensions of some sort, but which one you use depends on how important 
> > > the warning is. We've traditionally handled "use of whatever is a C++NN 
> > > extension" warnings as `ExtWarn` whereas the "use of whatever is 
> > > incompatible with C++ standards before C++NN" warnings are usually 
> > > `Extension`.
> > In this case, `compat_pp_directive` for C++2b & C2x uses `Warning`, but why 
> > did you choose it rather than `Extension`?
> Oh shoot, I misspoke and said `ExtWarn` when I meant `Warning` that's default 
> ignored. Sorry about that, and thank you for asking for clarification!
> 
> I picked that because it's the pattern used by many other diagnostic pairs:
> ```
> def ext_inline_variable : ExtWarn<
>   "inline variables are a C++17 extension">, InGroup;
> def warn_cxx14_compat_inline_variable : Warning<
>   "inline variables are incompatible with C++ standards before C++17">,
>   DefaultIgnore, InGroup;
> 
> def ext_for_range_begin_end_types_differ : ExtWarn<
>   "'begin' and 'end' returning different types (%0 and %1) is a C++17 
> extension">,
>   InGroup;
> def warn_for_range_begin_end_types_differ : Warning<
>   "'begin' and 'end' returning different types (%0 and %1) is incompatible "
>   "with C++ standards before C++17">, InGroup, DefaultIgnore;
> 
> def ext_constexpr_body_invalid_stmt : ExtWarn<
>   "use of this statement in a constexpr %select{function|constructor}0 "
>   "is a C++14 extension">, InGroup;
> def warn_cxx11_compat_constexpr_body_invalid_stmt : Warning<
>   "use of this statement in a constexpr %select{function|constructor}0 "
>   "is incompatible with C++ standards before C++14">,
>   InGroup, DefaultIgnore;
> 
> def ext_constexpr_function_try_block_cxx20 : ExtWarn<
>   "function try block in constexpr %select{function|constructor}0 is "
>   "a C++20 extension">, InGroup;
> def warn_cxx17_compat_constexpr_function_try_block : Warning<
>   "function try block in constexpr %select{function|constructor}0 is "
>   "incompatible with C++ standards before C++20">,
>   InGroup, DefaultIgnore;
> ```
Oh, that makes sense. Thank you for your explanation!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428395.
ken-matsui added a comment.

Fix the test for FindSimilarStr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive-c2x-cpp2b.c
  clang/test/Preprocessor/suggest-typoed-directive.c
  llvm/include/llvm/ADT/StringRef.h
  llvm/lib/Support/StringRef.cpp
  llvm/unittests/ADT/StringRefTest.cpp

Index: llvm/unittests/ADT/StringRefTest.cpp
===
--- llvm/unittests/ADT/StringRefTest.cpp
+++ llvm/unittests/ADT/StringRefTest.cpp
@@ -566,6 +566,13 @@
 }
 
 TEST(StringRefTest, EditDistance) {
+  EXPECT_EQ(0U, StringRef("").edit_distance(""));
+  EXPECT_EQ(1U, StringRef("").edit_distance("aaab"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("aacb"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("abca"));
+  EXPECT_EQ(3U, StringRef("aabc").edit_distance("adef"));
+  EXPECT_EQ(4U, StringRef("abcd").edit_distance("efgh"));
+
   StringRef Hello("hello");
   EXPECT_EQ(2U, Hello.edit_distance("hill"));
 
@@ -584,6 +591,28 @@
"people soiled our green "));
 }
 
+TEST(StringRefTest, FindSimilarStr) {
+  {
+std::vector Candidates{"b", "c"};
+EXPECT_EQ(None, StringRef("a").find_similar_str(Candidates));
+  }
+  { // macros
+std::vector Candidates{
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elfidef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(None, StringRef("special_compiler_directive").find_similar_str(Candidates));
+  }
+  { // case-insensitive
+std::vector Candidates{
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("ELIFDEF").find_similar_str(Candidates));
+  }
+}
+
 TEST(StringRefTest, Misc) {
   std::string Storage;
   raw_string_ostream OS(Storage);
Index: llvm/lib/Support/StringRef.cpp
===
--- llvm/lib/Support/StringRef.cpp
+++ llvm/lib/Support/StringRef.cpp
@@ -98,6 +98,43 @@
   AllowReplacements, MaxEditDistance);
 }
 
+// Find a similar string in `Candidates`.
+Optional StringRef::find_similar_str(const std::vector , size_t Dist) const {
+  // We need to check if `rng` has the exact case-insensitive string because the Levenshtein distance match does not
+  // care about it.
+  for (StringRef C : Candidates) {
+if (equals_insensitive(C)) {
+  return C.str();
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If dist is given, use the dist for maxDist; otherwise, if the LHS size is less than 3, use the LHS size minus 1
+  // and if not, use the LHS size divided by 3.
+  size_t MaxDist = Dist != 0? Dist
+   : Length < 3 ? Length - 1
+: Length / 3;
+
+  std::vector> Cand;
+  for (StringRef C : Candidates) {
+size_t CurDist = edit_distance(C, false);
+if (CurDist <= MaxDist) {
+  Cand.emplace_back(C, CurDist);
+}
+  }
+
+  if (Cand.empty()) {
+return None;
+  } else if (Cand.size() == 1) {
+return Cand[0].first;
+  } else {
+auto SimilarStr = std::min_element(
+Cand.cbegin(), Cand.cend(),
+[](const auto , const auto ) { return A.second < B.second; });
+return SimilarStr->first;
+  }
+}
+
 //===--===//
 // String Operations
 //===--===//
Index: llvm/include/llvm/ADT/StringRef.h
===
--- llvm/include/llvm/ADT/StringRef.h
+++ llvm/include/llvm/ADT/StringRef.h
@@ -10,6 +10,7 @@
 #define LLVM_ADT_STRINGREF_H
 
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Compiler.h"
@@ -24,6 +25,7 @@
 #endif
 #include 
 #include 
+#include 
 
 // Declare the __builtin_strlen intrinsic for MSVC so it can be used in
 // constexpr context.
@@ -240,6 +242,23 @@
 unsigned edit_distance(StringRef Other, bool AllowReplacements = true,
unsigned MaxEditDistance = 0) const;
 
+/// Find a similar string in `Candidates`.
+///
+/// \param Candidates the candidates to find a similar string.
+///
+/// \param Dist Maximum distance to elect as similar strings. Eventually,
+/// a 

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

In D125178#3503617 , @aaron.ballman 
wrote:

> It looks like precommit CI caught some related failures:
>
>   Failed Tests (7):
> Clang :: Lexer/deprecate-macro.c
> Clang :: Lexer/unsafe-macro.c
> Clang :: Preprocessor/elifdef.c
> Clang :: Preprocessor/if_warning.c
> Clang :: Preprocessor/ifdef-recover.c
> Clang :: Preprocessor/macro_misc.c
> Clang :: Preprocessor/macro_vaopt_check.cpp

Ahhh, my bad. Thank you for your heads up!




Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:696-698
+def ext_c2x_pp_directive : Extension<
+  "%select{#elifdef|#elifndef}0 is a C2x extension">,
+  InGroup;

aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > I think we need two diagnostics, one for C2x and one for C++2b 
> > > (https://wg21.link/p2334 was adopted for C++23). Each of these 
> > > diagnostics should come in a pair:
> > > ```
> > > def warn_cxx20_compat_pp_directive : Warning<
> > >   "use of a '#%select{elifdef|elifndef}0' directive is incompatible with 
> > > C++ standards before C++2b",
> > >   InGroup, DefaultIgnore;
> > > def ext_cxx20_pp_directive : ExtWarn<
> > >   "use of a '#%select{elifdef|elifndef}0' directive is a C++2b extension",
> > >   InGroup;
> > > ```
> > > and similar for C, except with wording about C standards and in the C 
> > > warning groups.
> > I thought I had to use `Extension` here, but what is the difference between 
> > `Warning`, `ExtWarn`, and `Extension`?
> Great question!
> 
> `Extension` diagnostics are warnings that are enabled via `-pedantic` but 
> otherwise off by default.
> `ExtWarn` diagnostics are warnings that are enabled via `-pedantic` but are 
> also on by default.
> `Warning` diagnostics are warnings. You can use things like `DefaultIgnore` 
> or `DefaultError` to control the default behavior of the warning.
> `Error` diagnostics are errors.
> 
> You'll typically use `Extension` or `ExtWarn` for things that are extensions 
> of some sort, but which one you use depends on how important the warning is. 
> We've traditionally handled "use of whatever is a C++NN extension" warnings 
> as `ExtWarn` whereas the "use of whatever is incompatible with C++ standards 
> before C++NN" warnings are usually `Extension`.
In this case, `compat_pp_directive` for C++2b & C2x uses `Warning`, but why did 
you choose it rather than `Extension`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

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

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428382.
ken-matsui added a comment.

Update test codes as reviewed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/line-directive-system-headers.c
  clang/test/Preprocessor/line-directive.c
  clang/test/SemaCXX/constexpr-string.cpp

Index: clang/test/SemaCXX/constexpr-string.cpp
===
--- clang/test/SemaCXX/constexpr-string.cpp
+++ clang/test/SemaCXX/constexpr-string.cpp
@@ -7,7 +7,7 @@
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 
-# 9 "/usr/include/string.h" 1 3 4
+# 9 "/usr/include/string.h" 1 3 4  // expected-warning {{this style of line directive is a GNU extension}}
 extern "C" {
   typedef decltype(sizeof(int)) size_t;
 
@@ -29,7 +29,7 @@
 }
 # 25 "SemaCXX/constexpr-string.cpp" 2
 
-# 27 "/usr/include/wchar.h" 1 3 4
+# 27 "/usr/include/wchar.h" 1 3 4  // expected-warning {{this style of line directive is a GNU extension}}
 extern "C" {
 #if NO_PREDEFINED_WCHAR_T
   typedef decltype(L'0') wchar_t;
Index: clang/test/Preprocessor/line-directive.c
===
--- clang/test/Preprocessor/line-directive.c
+++ clang/test/Preprocessor/line-directive.c
@@ -9,8 +9,8 @@
 # 20 "" 2
 
 // a push/pop before any other line control
-# 10 "enter-0" 1
-# 11 "" 2 // pop to main file
+# 10 "enter-0" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 11 "" 2 // pop to main file: expected-warning {{this style of line directive is a GNU extension}}
 #error MAIN7
 // expected-error@-1{{MAIN7}}
 
@@ -27,13 +27,15 @@
 #define A 42 "foo"
 #line A
 
-# 42
-# 42 "foo"
+# 42 // expected-warning {{this style of line directive is a GNU extension}}
+# 42 "foo" // expected-warning {{this style of line directive is a GNU extension}}
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
+// The next two lines do not get diagnosed because they are considered to be
+// within the system header, where diagnostics are suppressed.
 # 42 "foo" 1 3  // enter
 # 42 "foo" 2 3  // exit
 # 42 "foo" 2 3 4 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
-# 42 "foo" 3 4
+# 42 "foo" 3 4 // expected-warning {{this style of line directive is a GNU extension}}
 
 # 'a'// expected-error {{invalid preprocessing directive}}
 # 42 'f' // expected-error {{invalid filename for line marker directive}}
@@ -54,7 +56,7 @@
 
 // Verify that linemarker diddling of the system header flag works.
 
-# 192 "glomp.h" // not a system header.
+# 192 "glomp.h" // not a system header.: expected-warning {{this style of line directive is a GNU extension}}
 typedef int x;  // expected-note {{previous definition is here}}
 typedef int x;  // expected-warning {{redefinition of typedef 'x' is a C11 feature}}
 
@@ -97,7 +99,7 @@
 #line 010  // expected-warning {{#line directive interprets number as decimal, not octal}}
 extern int array[__LINE__ == 10 ? 1:-1];
 
-# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}}
+# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}} expected-warning {{this style of line directive is a GNU extension}}
 extern int array_gnuline[__LINE__ == 20 ? 1:-1];
 
 /* PR3917 */
@@ -106,7 +108,7 @@
 _\
 _LINE__ == 42 ? 1: -1];  /* line marker is location of first _ */
 
-# 51
+# 51 // expected-warning {{this style of line directive is a GNU extension}}
 extern char array2_gnuline[\
 _\
 _LINE__ == 52 ? 1: -1];  /* line marker is location of first _ */
@@ -115,12 +117,12 @@
 #line 0 "line-directive.c" // expected-warning {{#line directive with zero argument is a GNU extension}}
 undefined t; // expected-error {{unknown type name 'undefined'}}
 
-# 115 "main"
-# 116 "enter-1" 1
-# 117 "enter-2" 1
-# 118 "" 2 // pop to enter-1
+# 115 "main" // expected-warning {{this style of line directive is a GNU extension}}
+# 116 "enter-1" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 117 "enter-2" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 118 "" 2 // pop to enter-1: expected-warning {{this style of line directive is a GNU extension}}
 #error ENTER1
 // expected-error@-1{{ENTER1}}
-# 121 "" 2 // pop to "main"
+# 121 "" 2 // pop to "main": expected-warning {{this style of line directive is a GNU extension}}
 #error MAIN2
 // 

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you so much for your review!




Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:696-698
+def ext_c2x_pp_directive : Extension<
+  "%select{#elifdef|#elifndef}0 is a C2x extension">,
+  InGroup;

aaron.ballman wrote:
> I think we need two diagnostics, one for C2x and one for C++2b 
> (https://wg21.link/p2334 was adopted for C++23). Each of these diagnostics 
> should come in a pair:
> ```
> def warn_cxx20_compat_pp_directive : Warning<
>   "use of a '#%select{elifdef|elifndef}0' directive is incompatible with C++ 
> standards before C++2b",
>   InGroup, DefaultIgnore;
> def ext_cxx20_pp_directive : ExtWarn<
>   "use of a '#%select{elifdef|elifndef}0' directive is a C++2b extension",
>   InGroup;
> ```
> and similar for C, except with wording about C standards and in the C warning 
> groups.
I thought I had to use `Extension` here, but what is the difference between 
`Warning`, `ExtWarn`, and `Extension`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

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

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428358.
ken-matsui added a comment.

Update the code as reviewed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/ext-c2x-pp-directive.c
  clang/test/Preprocessor/ext-cpp2b-pp-directive.cpp

Index: clang/test/Preprocessor/ext-cpp2b-pp-directive.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/ext-cpp2b-pp-directive.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=pre-cpp2b-pedantic -pedantic %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=pre-cpp2b-compat -Wpre-c++2b-compat %s
+// RUN: not %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify %s
+
+int x;
+
+#if 1
+#elifdef A // #1
+#endif
+// pre-cpp2b-pedantic-warning@#1 {{use of a '#elifdef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C++ standards before C++2b}}
+
+#if 1
+#elifndef B // #2
+#endif
+// pre-cpp2b-pedantic-warning@#2 {{use of a '#elifndef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#2 {{use of a '#elifndef' directive is incompatible with C++ standards before C++2b}}
+
+#if 0
+#elifdef C
+#endif
+// pre-cpp2b-pedantic-warning@-2 {{use of a '#elifdef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@-3 {{use of a '#elifdef' directive is incompatible with C++ standards before C++2b}}
+
+#if 0
+#elifndef D
+#endif
+// pre-cpp2b-pedantic-warning@-2 {{use of a '#elifndef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@-3 {{use of a '#elifndef' directive is incompatible with C++ standards before C++2b}}
Index: clang/test/Preprocessor/ext-c2x-pp-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/ext-c2x-pp-directive.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=c99-pedantic -pedantic %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=pre-c2x-compat -Wpre-c2x-compat %s
+// RUN: not %clang_cc1 -std=c99 -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify %s
+
+int x;
+
+#if 1
+#elifdef A // #1
+#endif
+// c99-pedantic-warning@#1 {{use of a '#elifdef' directive is a C2x extension}}
+// pre-c2x-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C standards before C2x}}
+
+#if 1
+#elifndef B // #2
+#endif
+// c99-pedantic-warning@#2 {{use of a '#elifndef' directive is a C2x extension}}
+// pre-c2x-compat-warning@#2 {{use of a '#elifndef' directive is incompatible with C standards before C2x}}
+
+#if 0
+#elifdef C
+#endif
+// c99-pedantic-warning@-2 {{use of a '#elifdef' directive is a C2x extension}}
+// pre-c2x-compat-warning@-3 {{use of a '#elifdef' directive is incompatible with C standards before C2x}}
+
+#if 0
+#elifndef D
+#endif
+// c99-pedantic-warning@-2 {{use of a '#elifndef' directive is a C2x extension}}
+// pre-c2x-compat-warning@-3 {{use of a '#elifndef' directive is incompatible with C standards before C2x}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -652,6 +652,16 @@
 PPConditionalInfo  = CurPPLexer->peekConditionalLevel();
 Token DirectiveToken = Tok;
 
+unsigned DiagID;
+if (LangOpts.CPlusPlus)
+  DiagID = LangOpts.CPlusPlus2b ? diag::warn_cxx2b_compat_pp_directive
+: diag::ext_cxx2b_pp_directive;
+else
+  DiagID = LangOpts.C2x ? diag::warn_c2x_compat_pp_directive
+: diag::ext_c2x_pp_directive;
+
+Diag(Tok, DiagID) << (IsElifDef ? PED_Elifdef : PED_Elifndef) - 1;
+
 // If this is a #elif with a #else before it, report the error.
 if (CondInfo.FoundElse)
   Diag(Tok, diag::pp_err_elif_after_else)
@@ -3255,6 +3265,24 @@
  : PED_Elifndef;
   ++NumElse;
 
+  // Warn if using `#elifdef` & `#elifndef` in not C2x mode.
+  switch (DirKind) {
+  case PED_Elifdef:
+  case PED_Elifndef:
+unsigned DiagID;
+if (LangOpts.CPlusPlus)
+  DiagID = LangOpts.CPlusPlus2b ? diag::warn_cxx2b_compat_pp_directive
+: diag::ext_cxx2b_pp_directive;
+else
+  DiagID = LangOpts.C2x ? diag::warn_c2x_compat_pp_directive
+: diag::ext_c2x_pp_directive;
+
+Diag(ElifToken, DiagID) << DirKind - 1;
+break;
+  default:
+break;
+  }
+
   // 

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/test/Preprocessor/line-directive-system-headers.c:2-6
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'blonk.c:92:2: error: ABC'
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'blonk.c:93:2: error: DEF'
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'line-directive.c:11:2: error: MAIN7'
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'enter-1:118:2: error: ENTER1'
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'main:121:2: error: MAIN2'

aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > I think we're losing a bunch of test coverage from this. Why did you drop 
> > > these?
> > These are also tested on `line-directive.c` and possibly redundant, so I 
> > removed them.
> > Should I keep them?
> You're right, those are redundant; I'm fine removing them here.
I see :+1:


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you for your review!




Comment at: clang/test/Preprocessor/line-directive-system-headers.c:2-6
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'blonk.c:92:2: error: ABC'
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'blonk.c:93:2: error: DEF'
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'line-directive.c:11:2: error: MAIN7'
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'enter-1:118:2: error: ENTER1'
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'main:121:2: error: MAIN2'

aaron.ballman wrote:
> I think we're losing a bunch of test coverage from this. Why did you drop 
> these?
These are also tested on `line-directive.c` and possibly redundant, so I 
removed them.
Should I keep them?



Comment at: clang/test/Preprocessor/line-directive-system-headers.c:57-58
+# 192 "glomp.h" 3 // System header.: expected-warning {{this style of line 
directive is a GNU extension}}
+typedef int y;  // expected-note {{previous definition is here}}
+typedef int y;  // expected-warning {{redefinition of typedef 'y' is a C11 
feature}}
 

aaron.ballman wrote:
> It took me a moment, but we get these diagnostics because we asked for 
> warnings in a system header. So I think we're losing test coverage from this 
> change -- previously the test was checking that we suppressed the diagnostics 
> as expected.
> 
> I think you should have two RUN lines, one with `-Wsystem-headers` and one 
> without, so you can test the behavior of both situations. Note, you can do 
> `-verify=system` on the one with `-Wsystem-headers` so that you can write `// 
> system-warning {{whatever}}` on the diagnostics expected to only appear from 
> that RUN line.
> 
> Alternatively, you could make this test solely about system header behavior 
> and modify line-directive.c to not do anything with system headers. But I 
> think that's a much more invasive change.
Thank you! I'm going to edit this file, not `line-directive.c`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:444
+
+  if (auto Sugg = Directive.find_similar_str(Candidates)) {
+CharSourceRange DirectiveRange =

erichkeane wrote:
> I don't believe this meets our requirements for 'auto'. 
Thank you. I replaced it with the actual type.



Comment at: clang/test/Preprocessor/suggest-typoed-directive.c:10
+// expected-warning@+11 {{'#elfidef' directive not found, did you mean 
'#elifdef'?}}
+// expected-warning@+11 {{'#elfindef' directive not found, did you mean 
'#elifdef'?}}
+// expected-warning@+11 {{'#elsi' directive not found, did you mean '#else'?}}

aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > ken-matsui wrote:
> > > > aaron.ballman wrote:
> > > > > It's interesting that this one suggested `#elifdef` instead of 
> > > > > `#elifndef` -- is there anything that can be done for that?
> > > > > 
> > > > > Also, one somewhat interesting question is whether we want to 
> > > > > recommend `#elifdef` and `#elifndef` outside of C2x/C++2b mode. Those 
> > > > > directives only exist in the latest language standard, but Clang 
> > > > > supports them as a conforming extension in all language modes. Given 
> > > > > that this diagnostic is about typos, I think I'm okay suggesting the 
> > > > > directives even in older language modes. That's as likely to be a 
> > > > > correct suggestion as not, IMO.
> > > > > It's interesting that this one suggested `#elifdef` instead of 
> > > > > `#elifndef` -- is there anything that can be done for that?
> > > > 
> > > > I found I have to use `std::min_element` instead of `std::max_element` 
> > > > because we are finding the nearest (most minimum distance) string. 
> > > > Meanwhile, `#elfindef` has 2 distance with both `#elifdef` and 
> > > > `#elifndef`. After replacing `std::max_element` with 
> > > > `std::min_element`, I could suggest `#elifndef` from `#elfinndef`.
> > > > 
> > > > > Also, one somewhat interesting question is whether we want to 
> > > > > recommend `#elifdef` and `#elifndef` outside of C2x/C++2b mode. Those 
> > > > > directives only exist in the latest language standard, but Clang 
> > > > > supports them as a conforming extension in all language modes. Given 
> > > > > that this diagnostic is about typos, I think I'm okay suggesting the 
> > > > > directives even in older language modes. That's as likely to be a 
> > > > > correct suggestion as not, IMO.
> > > > 
> > > > I agree with you because Clang implements those directives, and the 
> > > > suggested code will also be compilable. I personally think only not 
> > > > compilable suggestions should be avoided. (Or, we might place 
> > > > additional info for outside of C2x/C++2b mode like `this is a C2x/C++2b 
> > > > feature but compilable on Clang`?)
> > > > 
> > > > ---
> > > > 
> > > > According to the algorithm of `std::min_element`, we only get an 
> > > > iterator of the first element even if there is another element that has 
> > > > the same distance. So, `#elfindef` only suggests `#elifdef` in 
> > > > accordance with the order of `Candidates`, and I don't think it is 
> > > > beautiful to depend on the order of candidates. I would say that we can 
> > > > suggest all the same distance like the following, but I'm not sure this 
> > > > is preferable:
> > > > 
> > > > ```
> > > > #elfindef // diag: unknown directive, did you mean #elifdef or 
> > > > #elifndef?
> > > > ```
> > > > 
> > > > I agree with you because Clang implements those directives, and the 
> > > > suggested code will also be compilable. I personally think only not 
> > > > compilable suggestions should be avoided. (Or, we might place 
> > > > additional info for outside of C2x/C++2b mode like this is a C2x/C++2b 
> > > > feature but compilable on Clang?)
> > > 
> > > I may be changing my mind on this a bit. I now see we don't issue an 
> > > extension warning when using `#elifdef` or `#elifndef` in older language 
> > > modes. That means suggesting those will be correct but only for Clang, 
> > > and the user won't have any other diagnostics to tell them about the 
> > > portability issue. And those particular macros are reasonably likely to 
> > > be used in a header where the user is trying to aim for portability. So 
> > > I'm starting to think we should only suggest those two in C2x mode (and 
> > > we should probably add a portability warning for #elifdef and #elifndef, 
> > > so I filed: https://github.com/llvm/llvm-project/issues/55306)
> > > 
> > > > I would say that we can suggest all the same distance like the 
> > > > following, but I'm not sure this is preferable:
> > > 
> > > The way we typically handle this is to attach FixIt hints to a note 
> > > instead of to the diagnostic. This way, automatic fixes aren't applied 
> > > (because there are multiple choices to pick from) but the user is still 
> > > able to apply whichever fix they want in 

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428335.
ken-matsui added a comment.
Herald added a subscriber: hiraditya.

Updated the code as reviewed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive-c2x-cpp2b.c
  clang/test/Preprocessor/suggest-typoed-directive.c
  llvm/include/llvm/ADT/StringRef.h
  llvm/lib/Support/StringRef.cpp
  llvm/unittests/ADT/StringRefTest.cpp

Index: llvm/unittests/ADT/StringRefTest.cpp
===
--- llvm/unittests/ADT/StringRefTest.cpp
+++ llvm/unittests/ADT/StringRefTest.cpp
@@ -566,6 +566,13 @@
 }
 
 TEST(StringRefTest, EditDistance) {
+  EXPECT_EQ(0U, StringRef("").edit_distance(""));
+  EXPECT_EQ(1U, StringRef("").edit_distance("aaab"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("aacb"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("abca"));
+  EXPECT_EQ(3U, StringRef("aabc").edit_distance("adef"));
+  EXPECT_EQ(4U, StringRef("abcd").edit_distance("efgh"));
+
   StringRef Hello("hello");
   EXPECT_EQ(2U, Hello.edit_distance("hill"));
 
@@ -584,6 +591,40 @@
"people soiled our green "));
 }
 
+TEST(StringRefTest, FindSimilarStr) {
+  {
+std::vector Candidates{"aaab", "aaabc"};
+EXPECT_EQ(std::string("aaab"), StringRef("").find_similar_str(Candidates));
+  }
+  {
+std::vector Candidates{"aab", "abc"};
+EXPECT_EQ(std::string("aab"), StringRef("aaa").find_similar_str(Candidates));
+  }
+  {
+std::vector Candidates{"ab", "bc"};
+EXPECT_EQ(std::string("ab"), StringRef("aa").find_similar_str(Candidates));
+  }
+  {
+std::vector Candidates{"b", "c"};
+EXPECT_EQ(None, StringRef("a").find_similar_str(Candidates));
+  }
+  { // macros
+std::vector Candidates{
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elfidef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(None, StringRef("special_compiler_directive").find_similar_str(Candidates));
+  }
+  { // case-insensitive
+std::vector Candidates{
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("ELIFDEF").find_similar_str(Candidates));
+  }
+}
+
 TEST(StringRefTest, Misc) {
   std::string Storage;
   raw_string_ostream OS(Storage);
Index: llvm/lib/Support/StringRef.cpp
===
--- llvm/lib/Support/StringRef.cpp
+++ llvm/lib/Support/StringRef.cpp
@@ -98,6 +98,43 @@
   AllowReplacements, MaxEditDistance);
 }
 
+// Find a similar string in `Candidates`.
+Optional StringRef::find_similar_str(const std::vector , size_t Dist) const {
+  // We need to check if `rng` has the exact case-insensitive string because the Levenshtein distance match does not
+  // care about it.
+  for (StringRef C : Candidates) {
+if (equals_insensitive(C)) {
+  return C.str();
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If dist is given, use the dist for maxDist; otherwise, if the LHS size is less than 3, use the LHS size minus 1
+  // and if not, use the LHS size divided by 3.
+  size_t MaxDist = Dist != 0? Dist
+   : Length < 3 ? Length - 1
+: Length / 3;
+
+  std::vector> Cand;
+  for (StringRef C : Candidates) {
+size_t CurDist = edit_distance(C, false);
+if (CurDist <= MaxDist) {
+  Cand.emplace_back(C, CurDist);
+}
+  }
+
+  if (Cand.empty()) {
+return None;
+  } else if (Cand.size() == 1) {
+return Cand[0].first;
+  } else {
+auto SimilarStr = std::min_element(
+Cand.cbegin(), Cand.cend(),
+[](const auto , const auto ) { return A.second < B.second; });
+return SimilarStr->first;
+  }
+}
+
 //===--===//
 // String Operations
 //===--===//
Index: llvm/include/llvm/ADT/StringRef.h
===
--- llvm/include/llvm/ADT/StringRef.h
+++ llvm/include/llvm/ADT/StringRef.h
@@ -10,6 +10,7 @@
 #define LLVM_ADT_STRINGREF_H
 
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Compiler.h"
@@ -24,6 +25,7 @@
 #endif
 #include 
 #include 
+#include 
 
 // Declare the __builtin_strlen 

[PATCH] D125078: Implement a feature to show line numbers in diagnostics

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

This feature is already implemented in GCC, so in my opinion, it would be great 
if we could turn this on by default.

I also added a feature to separate location info and a message in 
https://reviews.llvm.org/D125175.
Should its feature be avoided from turning on by default as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125078

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

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui marked an inline comment as not done.
ken-matsui added inline comments.



Comment at: clang/test/Preprocessor/line-directive.c:33
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop 
empty include stack}}
 # 42 "foo" 1 3  // enter
 # 42 "foo" 2 3  // exit

aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > Something is still not quite correct -- these should also be diagnosed as 
> > > an extension (it's the same feature just with flags).
> > This didn't cause a warning on this test file, but another test file I 
> > created caused a warning.
> > 
> > ```
> > // RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -pedantic %s
> > 
> > # 42 "foo" 1 3  // enter: expected-warning {{this style of line directive 
> > is a GNU extension}}
> > ```
> Oooo, wait a tick! This is entering a system header! Perhaps we're 
> silencing the warning because it's "in" a system header!
> 
> That's a neat edge case for us to think about -- do users of this diagnostic 
> *expect* such a construct to be diagnosed? What about the exit from the 
> system header? GCC seems to diagnose the entrance to the system header, but 
> not the exit: https://godbolt.org/z/PKGd4jh64 and I don't know if we want to 
> follow their behavior or not. Our current behavior is defensible, if it marks 
> the entrance to a system header or the exit from a system header, we're 
> silent. User who want to see the system header diagnostics can use 
> `-Wsystem-headers` to get them. So I think I've talked myself into the 
> current behavior here being correct -- but we should probably add a RUN line 
> that enables diagnostics in system headers to show our behavior there.
Ah, I'm quite not familiar with a preprocessor, so I couldn't have understood 
what was going on. Thank you for your clear explanation.

I've added a new file that tests with `-Wsystem-headers`. This test shows that 
it seems mostly correct expectations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428301.
ken-matsui added a comment.

Add a test file that uses the `-Wsystem-headers` option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/line-directive-system-headers.c
  clang/test/Preprocessor/line-directive.c
  clang/test/SemaCXX/constexpr-string.cpp

Index: clang/test/SemaCXX/constexpr-string.cpp
===
--- clang/test/SemaCXX/constexpr-string.cpp
+++ clang/test/SemaCXX/constexpr-string.cpp
@@ -7,7 +7,7 @@
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 
-# 9 "/usr/include/string.h" 1 3 4
+# 9 "/usr/include/string.h" 1 3 4  // expected-warning {{this style of line directive is a GNU extension}}
 extern "C" {
   typedef decltype(sizeof(int)) size_t;
 
@@ -29,7 +29,7 @@
 }
 # 25 "SemaCXX/constexpr-string.cpp" 2
 
-# 27 "/usr/include/wchar.h" 1 3 4
+# 27 "/usr/include/wchar.h" 1 3 4  // expected-warning {{this style of line directive is a GNU extension}}
 extern "C" {
 #if NO_PREDEFINED_WCHAR_T
   typedef decltype(L'0') wchar_t;
Index: clang/test/Preprocessor/line-directive.c
===
--- clang/test/Preprocessor/line-directive.c
+++ clang/test/Preprocessor/line-directive.c
@@ -9,8 +9,8 @@
 # 20 "" 2
 
 // a push/pop before any other line control
-# 10 "enter-0" 1
-# 11 "" 2 // pop to main file
+# 10 "enter-0" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 11 "" 2 // pop to main file: expected-warning {{this style of line directive is a GNU extension}}
 #error MAIN7
 // expected-error@-1{{MAIN7}}
 
@@ -27,13 +27,13 @@
 #define A 42 "foo"
 #line A
 
-# 42
-# 42 "foo"
+# 42 // expected-warning {{this style of line directive is a GNU extension}}
+# 42 "foo" // expected-warning {{this style of line directive is a GNU extension}}
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
 # 42 "foo" 1 3  // enter
 # 42 "foo" 2 3  // exit
 # 42 "foo" 2 3 4 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
-# 42 "foo" 3 4
+# 42 "foo" 3 4 // expected-warning {{this style of line directive is a GNU extension}}
 
 # 'a'// expected-error {{invalid preprocessing directive}}
 # 42 'f' // expected-error {{invalid filename for line marker directive}}
@@ -54,7 +54,7 @@
 
 // Verify that linemarker diddling of the system header flag works.
 
-# 192 "glomp.h" // not a system header.
+# 192 "glomp.h" // not a system header.: expected-warning {{this style of line directive is a GNU extension}}
 typedef int x;  // expected-note {{previous definition is here}}
 typedef int x;  // expected-warning {{redefinition of typedef 'x' is a C11 feature}}
 
@@ -97,7 +97,7 @@
 #line 010  // expected-warning {{#line directive interprets number as decimal, not octal}}
 extern int array[__LINE__ == 10 ? 1:-1];
 
-# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}}
+# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}} expected-warning {{this style of line directive is a GNU extension}}
 extern int array_gnuline[__LINE__ == 20 ? 1:-1];
 
 /* PR3917 */
@@ -106,7 +106,7 @@
 _\
 _LINE__ == 42 ? 1: -1];  /* line marker is location of first _ */
 
-# 51
+# 51 // expected-warning {{this style of line directive is a GNU extension}}
 extern char array2_gnuline[\
 _\
 _LINE__ == 52 ? 1: -1];  /* line marker is location of first _ */
@@ -115,12 +115,12 @@
 #line 0 "line-directive.c" // expected-warning {{#line directive with zero argument is a GNU extension}}
 undefined t; // expected-error {{unknown type name 'undefined'}}
 
-# 115 "main"
-# 116 "enter-1" 1
-# 117 "enter-2" 1
-# 118 "" 2 // pop to enter-1
+# 115 "main" // expected-warning {{this style of line directive is a GNU extension}}
+# 116 "enter-1" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 117 "enter-2" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 118 "" 2 // pop to enter-1: expected-warning {{this style of line directive is a GNU extension}}
 #error ENTER1
 // expected-error@-1{{ENTER1}}
-# 121 "" 2 // pop to "main"
+# 121 "" 2 // pop to "main": expected-warning {{this style of line directive is a GNU extension}}
 #error MAIN2
 // expected-error@-1{{MAIN2}}
Index: clang/test/Preprocessor/line-directive-system-headers.c

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui created this revision.
ken-matsui added a reviewer: aaron.ballman.
Herald added a project: All.
ken-matsui requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch resolves https://github.com/llvm/llvm-project/issues/55306.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125178

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/ext-c2x-pp-directive.c


Index: clang/test/Preprocessor/ext-c2x-pp-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/ext-c2x-pp-directive.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -std=c99 -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify %s
+
+#if 1
+#elifndef FOO // expected-warning {{#elifndef is a C2x extension}}
+#endif
+
+#if 1
+#elifdef BAR // expected-warning {{#elifdef is a C2x extension}}
+#endif
+
+// expected-warning {{ISO C requires a translation unit to contain at least 
one declaration}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -3255,6 +3255,17 @@
  : PED_Elifndef;
   ++NumElse;
 
+  // Warn if using `#elifdef` & `#elifndef` in not C2x mode.
+  switch (DirKind) {
+case PED_Elifdef:
+case PED_Elifndef:
+  if (!getLangOpts().C2x)
+Diag(ElifToken, diag::ext_c2x_pp_directive) << DirKind - 1;
+  break;
+default:
+  break;
+  }
+
   // #elif directive in a non-skipping conditional... start skipping.
   // We don't care what the condition is, because we will always skip it (since
   // the block immediately before it was included).
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -693,6 +693,9 @@
 def warn_cxx98_compat_pp_line_too_big : Warning<
   "#line number greater than 32767 is incompatible with C++98">,
   InGroup, DefaultIgnore;
+def ext_c2x_pp_directive : Extension<
+  "%select{#elifdef|#elifndef}0 is a C2x extension">,
+  InGroup;
 
 def err_pp_visibility_non_macro : Error<"no macro named %0">;
 


Index: clang/test/Preprocessor/ext-c2x-pp-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/ext-c2x-pp-directive.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -std=c99 -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify %s
+
+#if 1
+#elifndef FOO // expected-warning {{#elifndef is a C2x extension}}
+#endif
+
+#if 1
+#elifdef BAR // expected-warning {{#elifdef is a C2x extension}}
+#endif
+
+// expected-warning {{ISO C requires a translation unit to contain at least one declaration}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -3255,6 +3255,17 @@
  : PED_Elifndef;
   ++NumElse;
 
+  // Warn if using `#elifdef` & `#elifndef` in not C2x mode.
+  switch (DirKind) {
+case PED_Elifdef:
+case PED_Elifndef:
+  if (!getLangOpts().C2x)
+Diag(ElifToken, diag::ext_c2x_pp_directive) << DirKind - 1;
+  break;
+default:
+  break;
+  }
+
   // #elif directive in a non-skipping conditional... start skipping.
   // We don't care what the condition is, because we will always skip it (since
   // the block immediately before it was included).
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -693,6 +693,9 @@
 def warn_cxx98_compat_pp_line_too_big : Warning<
   "#line number greater than 32767 is incompatible with C++98">,
   InGroup, DefaultIgnore;
+def ext_c2x_pp_directive : Extension<
+  "%select{#elifdef|#elifndef}0 is a C2x extension">,
+  InGroup;
 
 def err_pp_visibility_non_macro : Error<"no macro named %0">;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:362-365
 
+def warn_pp_typo_directive : Warning<
+  "'#%0' directive not found, did you mean '#%1'?">,
+  InGroup;

aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > Rather than adding a warning over the top of an existing error, I think 
> > > we should modify `err_pp_invalid_directive` to have an optional "did you 
> > > mean?" when we find a plausible typo to correct.
> > > 
> > > However, we do not issue that diagnostic when it's inside of a skipped 
> > > conditional block, and that's what the crux of 
> > > https://github.com/llvm/llvm-project/issues/51598 is about. As @rsmith 
> > > pointed out in that issue (and I agree), compilers are free to support 
> > > custom directives and those will validly appear inside of a conditional 
> > > block that is skipped. We need to be careful not to diagnose those kinds 
> > > of situations as an error. However, we still want to diagnose when the 
> > > unknown directive is "sufficiently close" to another one which can 
> > > control the conditional chain. e.g.,
> > > ```
> > > #fi FOO // error unknown directive, did you mean #if?
> > > #endfi // error unknown directive, did you mean #endif?
> > > 
> > > #if FOO
> > > #esle // diag: unknown directive, did you mean #else?
> > > #elfi // diag: unknown directive, did you mean #elif?
> > > #elfidef // diag: unknown directive, did you mean #elifdef
> > > #elinfdef // diag: unknown directive, did you mean #elifndef
> > > 
> > > #frobble // No diagnostic, not close enough to a conditional directive to 
> > > warrant diagnosing
> > > #eerp // No diagnostic, not close enough to a conditional directive to 
> > > warrant diagnosing
> > > 
> > > #endif
> > > ```
> > > Today, if `FOO` is defined to a nonzero value, you'll get diagnostics for 
> > > all of those, but if `FOO` is not defined or is defined to 0, then 
> > > there's no diagnostics. I think we want to consider directives that are 
> > > *very likely* to be a typo (edit distance of 1, maybe 2?) for a 
> > > conditional directive as a special case.
> > > 
> > > Currently, we only diagnose unknown directives as an error. I think for 
> > > these special cased conditional directive diagnostics, we'll want to use 
> > > a warning rather than an error in this circumstance (just in case it 
> > > turns out to be a valid directive in a skipped conditional block). If we 
> > > do go that route and make it a warning, I think the warning group should 
> > > be `-Wunknown-directives` to mirror `-Wunknown-pragmas`, 
> > > `-Wunknown-attributes`, etc and it should be defined to have the same 
> > > text as the error case. e.g., 
> > > ```
> > > def err_pp_invalid_directive : Error<
> > >   "invalid preprocessing directive%select{|; did you mean '#%1'?}0"
> > > >;
> > > def warn_pp_invalid_directive : Warning<
> > >   err_pp_invalid_directive.Text>, 
> > > InGroup>;
> > > ```
> > > WDYT?
> > > 
> > > (These were my thoughts before seeing the rest of the patch. After 
> > > reading the patch, it looks like we have pretty similar ideas here, which 
> > > is great, but leaving the comment anyway in case you have further 
> > > opinions.)
> > > Currently, we only diagnose unknown directives as an error. I think for 
> > > these special cased conditional directive diagnostics, we'll want to use 
> > > a warning rather than an error in this circumstance (just in case it 
> > > turns out to be a valid directive in a skipped conditional block). If we 
> > > do go that route and make it a warning, I think the warning group should 
> > > be `-Wunknown-directives` to mirror `-Wunknown-pragmas`, 
> > > `-Wunknown-attributes`, etc and it should be defined to have the same 
> > > text as the error case. e.g., 
> > > ```
> > > def err_pp_invalid_directive : Error<
> > >   "invalid preprocessing directive%select{|; did you mean '#%1'?}0"
> > > >;
> > > def warn_pp_invalid_directive : Warning<
> > >   err_pp_invalid_directive.Text>, 
> > > InGroup>;
> > > ```
> > > WDYT?
> > > 
> > > (These were my thoughts before seeing the rest of the patch. After 
> > > reading the patch, it looks like we have pretty similar ideas here, which 
> > > is great, but leaving the comment anyway in case you have further 
> > > opinions.)
> > 
> > For now, I totally agree with deriving a new warning from 
> > `err_pp_invalid_directive`.
> > 
> > However, for future scalability, I think it would be great if we could 
> > split those diagnostics into Error & Warning and Help, for example. Rustc 
> > does split the diagnostics like the following, and I think this is quite 
> > clear. So, a bit apart from this patch, I speculate creating a diagnostic 
> > system that can split them would bring Clang diagnostics much more readable.
> > 
> > https://github.com/rust-lang/rust/blob/598d89bf142823b5d84e2eb0f0f9e418ee966a4b/src/test/ui/suggestions/suggest-trait-items.stderr

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

After some investigations, some directives behave weirdly.
I will continue investigating, but do you have any suggestions here?




Comment at: clang/test/Preprocessor/line-directive.c:33
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop 
empty include stack}}
 # 42 "foo" 1 3  // enter
 # 42 "foo" 2 3  // exit

aaron.ballman wrote:
> Something is still not quite correct -- these should also be diagnosed as an 
> extension (it's the same feature just with flags).
This didn't cause a warning on this test file, but another test file I created 
caused a warning.

```
// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -pedantic %s

# 42 "foo" 1 3  // enter: expected-warning {{this style of line directive is a 
GNU extension}}
```



Comment at: clang/test/Preprocessor/line-directive.c:61
 
 # 192 "glomp.h" 3 // System header.
 typedef int y;  // ok

aaron.ballman wrote:
> This should also be diagnosed.
This directive didn't cause a warning when marking both `# 192 "glomp.h"` and 
this. Marking only either directive works.

Works:
```
...
# 192 "glomp.h" // not a system header.: expected-warning {{this style of line 
directive is a GNU extension}}
typedef int x;  // expected-note {{previous definition is here}}
typedef int x;  // expected-warning {{redefinition of typedef 'x' is a C11 
feature}}

# 192 "glomp.h" 3 // System header.
...
```

Works:
```
...
# 192 "glomp.h" // not a system header.
typedef int x;  // expected-note {{previous definition is here}}
typedef int x;  // expected-warning {{redefinition of typedef 'x' is a C11 
feature}}

# 192 "glomp.h" 3 // System header.: expected-warning {{this style of line 
directive is a GNU extension}}
...
```

Does NOT work:
```
...
# 192 "glomp.h" // not a system header.: expected-warning {{this style of line 
directive is a GNU extension}}
typedef int x;  // expected-note {{previous definition is here}}
typedef int x;  // expected-warning {{redefinition of typedef 'x' is a C11 
feature}}

# 192 "glomp.h" 3 // System header.: expected-warning {{this style of line 
directive is a GNU extension}}
...
```

```
Command Output (stderr):
--
error: 'warning' diagnostics expected but not seen: 
  File /tmp/llvm/llvm-project/clang/test/Preprocessor/line-directive.c Line 
191: this style of line directive is a GNU extension
1 error generated.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@aaron.ballman

Thank you for your review and sorry to have missed those directives!

I also found additional directives that I might have to diagnose.
Could you please tell me if these are also required?




Comment at: clang/test/Preprocessor/line-directive.c:67-77
 #line 42 "blonk.h"  // doesn't change system headerness.
 
 typedef int z;  // ok
 typedef int z;  // ok
 
 # 97 // doesn't change system headerness.
 

Should these directives also be diagnosed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D125078: Implement a feature to show line numbers in diagnostics

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 427546.
ken-matsui added a comment.

Remove unnecessary includes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125078

Files:
  clang/include/clang/Basic/DiagnosticOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
  clang/test/FixIt/fixit-newline-style.c
  clang/test/FixIt/fixit-unicode-with-utf8-output.c
  clang/test/FixIt/fixit-unicode.c
  clang/test/Frontend/diag-show-line-numbers.c
  clang/test/Frontend/source-col-map.c
  clang/test/Lexer/header.cpp
  clang/test/Lexer/string-literal-errors.cpp
  clang/test/Misc/caret-diags-macros.c
  clang/test/Misc/caret-diags-multiline.cpp
  clang/test/Misc/diag-macro-backtrace.c
  clang/test/Misc/message-length.c
  clang/test/Misc/tabstop.c
  clang/test/Misc/unnecessary-elipses.cpp
  clang/test/Misc/unprintable.c
  clang/test/Misc/wrong-encoding.c
  clang/test/Parser/brackets.c
  clang/test/Parser/brackets.cpp
  clang/test/Preprocessor/ucn-pp-identifier.c
  clang/test/SemaCXX/struct-class-redecl.cpp

Index: clang/test/SemaCXX/struct-class-redecl.cpp
===
--- clang/test/SemaCXX/struct-class-redecl.cpp
+++ clang/test/SemaCXX/struct-class-redecl.cpp
@@ -87,113 +87,113 @@
 /*
 *** 'X' messages ***
 CHECK: warning: struct 'X' was previously declared as a class
-CHECK: {{^}}typedef struct X * X_t;
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}4 | typedef struct X * X_t;
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: previous use is here
-CHECK: {{^}}class X;
-CHECK: {{^}}  ^{{$}}
+CHECK: {{^}}3 | class X;
+CHECK: {{^}}  |   ^{{$}}
 CHECK: error: use of 'X' with tag type that does not match previous declaration
-CHECK: {{^}}union X { int x; float y; };
-CHECK: {{^}}^{{$}}
-CHECK: {{^}}class{{$}}
+CHECK: {{^}}5 | union X { int x; float y; };
+CHECK: {{^}}  | ^{{$}}
+CHECK: {{^}}  | class{{$}}
 CHECK: note: previous use is here
-CHECK: {{^}}class X;
-CHECK: {{^}}  ^{{$}}
+CHECK: {{^}}3 | class X;
+CHECK: {{^}}  |   ^{{$}}
 *** 'Y' messages ***
 CHECK: warning: 'Y' defined as a class template here but
   previously declared as a struct template
-CHECK: {{^}}template class Y { };
-CHECK: {{^}}  ^{{$}}
+CHECK: {{^}}8 | template class Y { };
+CHECK: {{^}}  |   ^{{$}}
 CHECK: note: did you mean class here?
-CHECK: {{^}}template struct Y;
-CHECK: {{^}} ^~{{$}}
-CHECK: {{^}} class{{$}}
+CHECK: {{^}}7 | template struct Y;
+CHECK: {{^}}  |  ^~{{$}}
+CHECK: {{^}}  |  class{{$}}
 *** 'A' messages ***
 CHECK: warning: struct 'A' was previously declared as a class
-CHECK: {{^}}struct A;
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}   18 | struct A;
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: previous use is here
-CHECK: {{^}}class A;
-CHECK: {{^}}  ^{{$}}
+CHECK: {{^}}   17 | class A;
+CHECK: {{^}}  |   ^{{$}}
 *** 'B' messages ***
 CHECK: warning: struct 'B' was previously declared as a class
-CHECK: {{^}}struct B;
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}   23 | struct B;
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: previous use is here
-CHECK: {{^}}class B;
-CHECK: {{^}}  ^{{$}}
+CHECK: {{^}}   21 | class B;
+CHECK: {{^}}  |   ^{{$}}
 CHECK: 'B' defined as a struct here but previously declared as a class
-CHECK: {{^}}struct B {};
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}   24 | struct B {};
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: did you mean struct here?
-CHECK: {{^}}class B;
-CHECK: {{^}}^{{$}}
-CHECK: {{^}}struct{{$}}
+CHECK: {{^}}   21 | class B;
+CHECK: {{^}}  | ^{{$}}
+CHECK: {{^}}  | struct{{$}}
 CHECK: note: did you mean struct here?
-CHECK: {{^}}class B;
-CHECK: {{^}}^{{$}}
-CHECK: {{^}}struct{{$}}
+CHECK: {{^}}   20 | class B;
+CHECK: {{^}}  | ^{{$}}
+CHECK: {{^}}  | struct{{$}}
 *** 'C' messages ***
 CHECK: warning: struct 'C' was previously declared as a class
-CHECK: {{^}}struct C;
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}   27 | struct C;
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: previous use is here
-CHECK: {{^}}class C;
-CHECK: {{^}}  ^{{$}}
+CHECK: {{^}}   26 | class C;
+CHECK: {{^}}  |   ^{{$}}
 CHECK: warning: class 'C' was previously declared as a struct
-CHECK: {{^}}class C;
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}   30 | class C;
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: previous use is here
-CHECK: {{^}}struct C;
-CHECK: {{^}}   ^{{$}}
+CHECK: {{^}}   27 | struct C;
+CHECK: {{^}}  |^{{$}}
 CHECK: warning: struct 'C' was previously declared as a class
-CHECK: {{^}}struct C;
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}   32 | struct C;
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: previous use is here
-CHECK: 

[PATCH] D125078: Implement a feature to show line numbers in diagnostics

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui created this revision.
Herald added a project: All.
ken-matsui requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

I implemented a feature to show line numbers with left margins in diagnostics 
like GCC-9 does. Ref: 
https://developers.redhat.com/blog/2019/03/08/usability-improvements-in-gcc-9


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125078

Files:
  clang/include/clang/Basic/DiagnosticOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
  clang/test/FixIt/fixit-newline-style.c
  clang/test/FixIt/fixit-unicode-with-utf8-output.c
  clang/test/FixIt/fixit-unicode.c
  clang/test/Frontend/diag-show-line-numbers.c
  clang/test/Frontend/source-col-map.c
  clang/test/Lexer/header.cpp
  clang/test/Lexer/string-literal-errors.cpp
  clang/test/Misc/caret-diags-macros.c
  clang/test/Misc/caret-diags-multiline.cpp
  clang/test/Misc/diag-macro-backtrace.c
  clang/test/Misc/message-length.c
  clang/test/Misc/tabstop.c
  clang/test/Misc/unnecessary-elipses.cpp
  clang/test/Misc/unprintable.c
  clang/test/Misc/wrong-encoding.c
  clang/test/Parser/brackets.c
  clang/test/Parser/brackets.cpp
  clang/test/Preprocessor/ucn-pp-identifier.c
  clang/test/SemaCXX/struct-class-redecl.cpp

Index: clang/test/SemaCXX/struct-class-redecl.cpp
===
--- clang/test/SemaCXX/struct-class-redecl.cpp
+++ clang/test/SemaCXX/struct-class-redecl.cpp
@@ -87,113 +87,113 @@
 /*
 *** 'X' messages ***
 CHECK: warning: struct 'X' was previously declared as a class
-CHECK: {{^}}typedef struct X * X_t;
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}4 | typedef struct X * X_t;
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: previous use is here
-CHECK: {{^}}class X;
-CHECK: {{^}}  ^{{$}}
+CHECK: {{^}}3 | class X;
+CHECK: {{^}}  |   ^{{$}}
 CHECK: error: use of 'X' with tag type that does not match previous declaration
-CHECK: {{^}}union X { int x; float y; };
-CHECK: {{^}}^{{$}}
-CHECK: {{^}}class{{$}}
+CHECK: {{^}}5 | union X { int x; float y; };
+CHECK: {{^}}  | ^{{$}}
+CHECK: {{^}}  | class{{$}}
 CHECK: note: previous use is here
-CHECK: {{^}}class X;
-CHECK: {{^}}  ^{{$}}
+CHECK: {{^}}3 | class X;
+CHECK: {{^}}  |   ^{{$}}
 *** 'Y' messages ***
 CHECK: warning: 'Y' defined as a class template here but
   previously declared as a struct template
-CHECK: {{^}}template class Y { };
-CHECK: {{^}}  ^{{$}}
+CHECK: {{^}}8 | template class Y { };
+CHECK: {{^}}  |   ^{{$}}
 CHECK: note: did you mean class here?
-CHECK: {{^}}template struct Y;
-CHECK: {{^}} ^~{{$}}
-CHECK: {{^}} class{{$}}
+CHECK: {{^}}7 | template struct Y;
+CHECK: {{^}}  |  ^~{{$}}
+CHECK: {{^}}  |  class{{$}}
 *** 'A' messages ***
 CHECK: warning: struct 'A' was previously declared as a class
-CHECK: {{^}}struct A;
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}   18 | struct A;
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: previous use is here
-CHECK: {{^}}class A;
-CHECK: {{^}}  ^{{$}}
+CHECK: {{^}}   17 | class A;
+CHECK: {{^}}  |   ^{{$}}
 *** 'B' messages ***
 CHECK: warning: struct 'B' was previously declared as a class
-CHECK: {{^}}struct B;
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}   23 | struct B;
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: previous use is here
-CHECK: {{^}}class B;
-CHECK: {{^}}  ^{{$}}
+CHECK: {{^}}   21 | class B;
+CHECK: {{^}}  |   ^{{$}}
 CHECK: 'B' defined as a struct here but previously declared as a class
-CHECK: {{^}}struct B {};
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}   24 | struct B {};
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: did you mean struct here?
-CHECK: {{^}}class B;
-CHECK: {{^}}^{{$}}
-CHECK: {{^}}struct{{$}}
+CHECK: {{^}}   21 | class B;
+CHECK: {{^}}  | ^{{$}}
+CHECK: {{^}}  | struct{{$}}
 CHECK: note: did you mean struct here?
-CHECK: {{^}}class B;
-CHECK: {{^}}^{{$}}
-CHECK: {{^}}struct{{$}}
+CHECK: {{^}}   20 | class B;
+CHECK: {{^}}  | ^{{$}}
+CHECK: {{^}}  | struct{{$}}
 *** 'C' messages ***
 CHECK: warning: struct 'C' was previously declared as a class
-CHECK: {{^}}struct C;
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}   27 | struct C;
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: previous use is here
-CHECK: {{^}}class C;
-CHECK: {{^}}  ^{{$}}
+CHECK: {{^}}   26 | class C;
+CHECK: {{^}}  |   ^{{$}}
 CHECK: warning: class 'C' was previously declared as a struct
-CHECK: {{^}}class C;
-CHECK: {{^}}^{{$}}
+CHECK: {{^}}   30 | class C;
+CHECK: {{^}}  | ^{{$}}
 CHECK: note: previous use is here
-CHECK: {{^}}struct C;
-CHECK: {{^}}   ^{{$}}
+CHECK: {{^}}   27 | struct C;
+CHECK: {{^}}  |^{{$}}
 CHECK: 

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@aaron.ballman

I could suppress the strange warnings by using `isWrittenInBuiltinFile` and 
`isWrittenInCommandLineFile`. Thank you!
Could you please review this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 427482.
ken-matsui added a comment.

Update codes as reviewed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/line-directive.c
  clang/test/SemaCXX/constexpr-string.cpp

Index: clang/test/SemaCXX/constexpr-string.cpp
===
--- clang/test/SemaCXX/constexpr-string.cpp
+++ clang/test/SemaCXX/constexpr-string.cpp
@@ -7,7 +7,7 @@
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 
-# 9 "/usr/include/string.h" 1 3 4
+# 9 "/usr/include/string.h" 1 3 4  // expected-warning {{this style of line directive is a GNU extension}}
 extern "C" {
   typedef decltype(sizeof(int)) size_t;
 
@@ -29,7 +29,7 @@
 }
 # 25 "SemaCXX/constexpr-string.cpp" 2
 
-# 27 "/usr/include/wchar.h" 1 3 4
+# 27 "/usr/include/wchar.h" 1 3 4  // expected-warning {{this style of line directive is a GNU extension}}
 extern "C" {
 #if NO_PREDEFINED_WCHAR_T
   typedef decltype(L'0') wchar_t;
Index: clang/test/Preprocessor/line-directive.c
===
--- clang/test/Preprocessor/line-directive.c
+++ clang/test/Preprocessor/line-directive.c
@@ -9,8 +9,8 @@
 # 20 "" 2
 
 // a push/pop before any other line control
-# 10 "enter-0" 1
-# 11 "" 2 // pop to main file
+# 10 "enter-0" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 11 "" 2 // pop to main file: expected-warning {{this style of line directive is a GNU extension}}
 #error MAIN7
 // expected-error@-1{{MAIN7}}
 
@@ -27,13 +27,13 @@
 #define A 42 "foo"
 #line A
 
-# 42
-# 42 "foo"
+# 42 // expected-warning {{this style of line directive is a GNU extension}}
+# 42 "foo" // expected-warning {{this style of line directive is a GNU extension}}
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
 # 42 "foo" 1 3  // enter
 # 42 "foo" 2 3  // exit
 # 42 "foo" 2 3 4 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
-# 42 "foo" 3 4
+# 42 "foo" 3 4 // expected-warning {{this style of line directive is a GNU extension}}
 
 # 'a'// expected-error {{invalid preprocessing directive}}
 # 42 'f' // expected-error {{invalid filename for line marker directive}}
@@ -54,7 +54,7 @@
 
 // Verify that linemarker diddling of the system header flag works.
 
-# 192 "glomp.h" // not a system header.
+# 192 "glomp.h" // not a system header.: expected-warning {{this style of line directive is a GNU extension}}
 typedef int x;  // expected-note {{previous definition is here}}
 typedef int x;  // expected-warning {{redefinition of typedef 'x' is a C11 feature}}
 
@@ -97,7 +97,7 @@
 #line 010  // expected-warning {{#line directive interprets number as decimal, not octal}}
 extern int array[__LINE__ == 10 ? 1:-1];
 
-# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}}
+# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}} expected-warning {{this style of line directive is a GNU extension}}
 extern int array_gnuline[__LINE__ == 20 ? 1:-1];
 
 /* PR3917 */
@@ -106,7 +106,7 @@
 _\
 _LINE__ == 42 ? 1: -1];  /* line marker is location of first _ */
 
-# 51
+# 51 // expected-warning {{this style of line directive is a GNU extension}}
 extern char array2_gnuline[\
 _\
 _LINE__ == 52 ? 1: -1];  /* line marker is location of first _ */
@@ -115,12 +115,12 @@
 #line 0 "line-directive.c" // expected-warning {{#line directive with zero argument is a GNU extension}}
 undefined t; // expected-error {{unknown type name 'undefined'}}
 
-# 115 "main"
-# 116 "enter-1" 1
-# 117 "enter-2" 1
-# 118 "" 2 // pop to enter-1
+# 115 "main" // expected-warning {{this style of line directive is a GNU extension}}
+# 116 "enter-1" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 117 "enter-2" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 118 "" 2 // pop to enter-1: expected-warning {{this style of line directive is a GNU extension}}
 #error ENTER1
 // expected-error@-1{{ENTER1}}
-# 121 "" 2 // pop to "main"
+# 121 "" 2 // pop to "main": expected-warning {{this style of line directive is a GNU extension}}
 #error MAIN2
 // expected-error@-1{{MAIN2}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1431,6 +1431,7 @@
   // 

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 427263.
ken-matsui added a comment.

Remove unnecessary includes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c
  llvm/include/llvm/ADT/StringRef.h
  llvm/unittests/ADT/StringRefTest.cpp

Index: llvm/unittests/ADT/StringRefTest.cpp
===
--- llvm/unittests/ADT/StringRefTest.cpp
+++ llvm/unittests/ADT/StringRefTest.cpp
@@ -566,6 +566,13 @@
 }
 
 TEST(StringRefTest, EditDistance) {
+  EXPECT_EQ(0U, StringRef("").edit_distance(""));
+  EXPECT_EQ(1U, StringRef("").edit_distance("aaab"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("aacb"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("abca"));
+  EXPECT_EQ(3U, StringRef("aabc").edit_distance("adef"));
+  EXPECT_EQ(4U, StringRef("abcd").edit_distance("efgh"));
+
   StringRef Hello("hello");
   EXPECT_EQ(2U, Hello.edit_distance("hill"));
 
@@ -584,6 +591,40 @@
"people soiled our green "));
 }
 
+TEST(StringRefTest, FindSimilarStr) {
+  {
+constexpr StringRef Candidates[] = {"aaab", "aaabc"};
+EXPECT_EQ(std::string("aaab"), StringRef("").find_similar_str(Candidates));
+  }
+  {
+constexpr StringRef Candidates[] = {"aab", "abc"};
+EXPECT_EQ(std::string("aab"), StringRef("aaa").find_similar_str(Candidates));
+  }
+  {
+constexpr StringRef Candidates[] = {"ab", "bc"};
+EXPECT_EQ(std::string("ab"), StringRef("aa").find_similar_str(Candidates));
+  }
+  {
+constexpr StringRef Candidates[] = {"b", "c"};
+EXPECT_EQ(None, StringRef("a").find_similar_str(Candidates));
+  }
+  { // macros
+constexpr StringRef Candidates[] = {
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elfidef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(None, StringRef("special_compiler_directive").find_similar_str(Candidates));
+  }
+  { // case-insensitive
+constexpr StringRef Candidates[] = {
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("ELIFDEF").find_similar_str(Candidates));
+  }
+}
+
 TEST(StringRefTest, Misc) {
   std::string Storage;
   raw_string_ostream OS(Storage);
Index: llvm/include/llvm/ADT/StringRef.h
===
--- llvm/include/llvm/ADT/StringRef.h
+++ llvm/include/llvm/ADT/StringRef.h
@@ -10,6 +10,7 @@
 #define LLVM_ADT_STRINGREF_H
 
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Compiler.h"
@@ -24,6 +25,7 @@
 #endif
 #include 
 #include 
+#include 
 
 // Declare the __builtin_strlen intrinsic for MSVC so it can be used in
 // constexpr context.
@@ -240,6 +242,46 @@
 unsigned edit_distance(StringRef Other, bool AllowReplacements = true,
unsigned MaxEditDistance = 0) const;
 
+/// Find a similar string in `Candidates`.
+template 
+Optional find_similar_str(const StringRef ()[Size],
+   size_t Dist = 0) const {
+  // We need to check if `rng` has the exact case-insensitive string because the
+  // Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (equals_insensitive(C)) {
+  return C.str();
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If dist is given, use the dist for maxDist; otherwise, if word size is
+  // less than 3, use the word size minus 1 and if not, use the word size
+  // divided by 3.
+  size_t MaxDist = Dist != 0? Dist
+   : Length < 3 ? Length - 1
+: Length / 3;
+
+  std::vector> Cand;
+  for (StringRef C : Candidates) {
+size_t CurDist = edit_distance(C, false);
+if (CurDist <= MaxDist) {
+  Cand.emplace_back(C, CurDist);
+}
+  }
+
+  if (Cand.empty()) {
+return None;
+  } else if (Cand.size() == 1) {
+return Cand[0].first;
+  } else {
+auto SimilarStr = std::min_element(
+Cand.cbegin(), Cand.cend(),
+[](const auto , const auto ) { return A.second < B.second; });
+return SimilarStr->first;
+  }
+}
+
 /// str - Get the contents as an std::string.
 

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 427262.
ken-matsui added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Update codes as reviewed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c
  llvm/include/llvm/ADT/StringRef.h
  llvm/unittests/ADT/StringRefTest.cpp

Index: llvm/unittests/ADT/StringRefTest.cpp
===
--- llvm/unittests/ADT/StringRefTest.cpp
+++ llvm/unittests/ADT/StringRefTest.cpp
@@ -566,6 +566,13 @@
 }
 
 TEST(StringRefTest, EditDistance) {
+  EXPECT_EQ(0U, StringRef("").edit_distance(""));
+  EXPECT_EQ(1U, StringRef("").edit_distance("aaab"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("aacb"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("abca"));
+  EXPECT_EQ(3U, StringRef("aabc").edit_distance("adef"));
+  EXPECT_EQ(4U, StringRef("abcd").edit_distance("efgh"));
+
   StringRef Hello("hello");
   EXPECT_EQ(2U, Hello.edit_distance("hill"));
 
@@ -584,6 +591,40 @@
"people soiled our green "));
 }
 
+TEST(StringRefTest, FindSimilarStr) {
+  {
+constexpr StringRef Candidates[] = {"aaab", "aaabc"};
+EXPECT_EQ(std::string("aaab"), StringRef("").find_similar_str(Candidates));
+  }
+  {
+constexpr StringRef Candidates[] = {"aab", "abc"};
+EXPECT_EQ(std::string("aab"), StringRef("aaa").find_similar_str(Candidates));
+  }
+  {
+constexpr StringRef Candidates[] = {"ab", "bc"};
+EXPECT_EQ(std::string("ab"), StringRef("aa").find_similar_str(Candidates));
+  }
+  {
+constexpr StringRef Candidates[] = {"b", "c"};
+EXPECT_EQ(None, StringRef("a").find_similar_str(Candidates));
+  }
+  { // macros
+constexpr StringRef Candidates[] = {
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elfidef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(None, StringRef("special_compiler_directive").find_similar_str(Candidates));
+  }
+  { // case-insensitive
+constexpr StringRef Candidates[] = {
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("ELIFDEF").find_similar_str(Candidates));
+  }
+}
+
 TEST(StringRefTest, Misc) {
   std::string Storage;
   raw_string_ostream OS(Storage);
Index: llvm/include/llvm/ADT/StringRef.h
===
--- llvm/include/llvm/ADT/StringRef.h
+++ llvm/include/llvm/ADT/StringRef.h
@@ -10,6 +10,7 @@
 #define LLVM_ADT_STRINGREF_H
 
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Compiler.h"
@@ -24,6 +25,7 @@
 #endif
 #include 
 #include 
+#include 
 
 // Declare the __builtin_strlen intrinsic for MSVC so it can be used in
 // constexpr context.
@@ -240,6 +242,46 @@
 unsigned edit_distance(StringRef Other, bool AllowReplacements = true,
unsigned MaxEditDistance = 0) const;
 
+/// Find a similar string in `Candidates`.
+template 
+Optional find_similar_str(const StringRef ()[Size],
+   size_t Dist = 0) const {
+  // We need to check if `rng` has the exact case-insensitive string because the
+  // Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (equals_insensitive(C)) {
+  return C.str();
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If dist is given, use the dist for maxDist; otherwise, if word size is
+  // less than 3, use the word size minus 1 and if not, use the word size
+  // divided by 3.
+  size_t MaxDist = Dist != 0? Dist
+   : Length < 3 ? Length - 1
+: Length / 3;
+
+  std::vector> Cand;
+  for (StringRef C : Candidates) {
+size_t CurDist = edit_distance(C, false);
+if (CurDist <= MaxDist) {
+  Cand.emplace_back(C, CurDist);
+}
+  }
+
+  if (Cand.empty()) {
+return None;
+  } else if (Cand.size() == 1) {
+return Cand[0].first;
+  } else {
+auto SimilarStr = std::min_element(
+Cand.cbegin(), Cand.cend(),
+[](const auto , const auto ) { return A.second < B.second; });
+return SimilarStr->first;
+  }
+}
+
 

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/include/clang/Tooling/LevDistance.h:1
+//===--- LevDistance.h --*- C++ 
-*-===//
+//

ken-matsui wrote:
> aaron.ballman wrote:
> > You shouldn't need this file or the implementation file -- we have this 
> > functionality already in: 
> > https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/edit_distance.h
> >  and 
> > https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/StringRef.h#L240.
> I totally missed the implementations! Sorry.
> 
> But where should I put both `findSimilarStr` & `findSimilarStr`?
> It seems that their same implementations aren't there.
@aaron.ballman 

The computation results are not matched with my implementation and another 
language implementation.

So, several directives that could be suggested in my implementation are no 
longer possible to be suggested, such as `#id` -> `#if`, `#elid` -> `#elif`, 
and `#elsi` -> `#else`, when using `llvm::ComputeEditDistance`.

The implementation of `llvm::ComputeEditDistance` might be also correct, but 
some distances seem to be calculated longer, which causes fewer suggestions.

Should I keep going with this implementation or not?

---

[llvm::ComputeEditDistance](https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/edit_distance.h):

```
size_t Dist = Str1.edit_distance(Str2, false);
```

```
Str1: id, Str2: if, Dist: 2 # <-- here
Str1: id, Str2: ifdef, Dist: 3
Str1: id, Str2: ifndef, Dist: 4
Str1: ifd, Str2: if, Dist: 1
Str1: ifd, Str2: ifdef, Dist: 2
Str1: ifd, Str2: ifndef, Dist: 3
```

[My implementation](https://wandbox.org/permlink/zRjT41alOHdwcf00):

```
Str1: id, Str2: if, Dist: 1 # <-- here
Str1: id, Str2: ifdef, Dist: 3
Str1: id, Str2: ifndef, Dist: 4
Str1: ifd, Str2: if, Dist: 1
Str1: ifd, Str2: ifdef, Dist: 2
Str1: ifd, Str2: ifndef, Dist: 3
```

[Rustc implementation](https://wandbox.org/permlink/08SB08JFF5G4awx4):

```
Str1: id, Str2: if, Dist: 1 # <-- here
Str1: id, Str2: ifdef, Dist: 3
Str1: id, Str2: ifndef, Dist: 4
Str1: ifd, Str2: if, Dist: 1
Str1: ifd, Str2: ifdef, Dist: 2
Str1: ifd, Str2: ifndef, Dist: 3
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

In D124534#3490732 , @aaron.ballman 
wrote:

> The tests run in -cc1 mode and don't #include anything, so I don't think the 
> issue is an internally included SDK. I think the issue could be from this: 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/InitPreprocessor.cpp#L1355
>  and 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/InitPreprocessor.cpp#L1368.
>  You may have to hook up to a debugger to see why we're issuing those 
> surprising warnings. If it turns out that it's these inserted directives, you 
> may have to look at the source location of the digit token to see if 
> `isWrittenInBuiltinFile()` is true or not (and we may need to also check 
> `isWrittenInScratchSpace()` as well, perhaps).

Thank you for the information! I'm going to look into the source code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you so much for your clear review!




Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1023-1024
 
+// Typoed directive warnings
+def TypoedDirective : DiagGroup<"typoed-directive">;
+

aaron.ballman wrote:
> We don't typically use "typo" in the user-facing part of diagnostics and this 
> group doesn't seem likely to be reused, so I would remove it (another comment 
> on this elsewhere).
Ah, I see. Thank you!



Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:362-365
 
+def warn_pp_typo_directive : Warning<
+  "'#%0' directive not found, did you mean '#%1'?">,
+  InGroup;

aaron.ballman wrote:
> Rather than adding a warning over the top of an existing error, I think we 
> should modify `err_pp_invalid_directive` to have an optional "did you mean?" 
> when we find a plausible typo to correct.
> 
> However, we do not issue that diagnostic when it's inside of a skipped 
> conditional block, and that's what the crux of 
> https://github.com/llvm/llvm-project/issues/51598 is about. As @rsmith 
> pointed out in that issue (and I agree), compilers are free to support custom 
> directives and those will validly appear inside of a conditional block that 
> is skipped. We need to be careful not to diagnose those kinds of situations 
> as an error. However, we still want to diagnose when the unknown directive is 
> "sufficiently close" to another one which can control the conditional chain. 
> e.g.,
> ```
> #fi FOO // error unknown directive, did you mean #if?
> #endfi // error unknown directive, did you mean #endif?
> 
> #if FOO
> #esle // diag: unknown directive, did you mean #else?
> #elfi // diag: unknown directive, did you mean #elif?
> #elfidef // diag: unknown directive, did you mean #elifdef
> #elinfdef // diag: unknown directive, did you mean #elifndef
> 
> #frobble // No diagnostic, not close enough to a conditional directive to 
> warrant diagnosing
> #eerp // No diagnostic, not close enough to a conditional directive to 
> warrant diagnosing
> 
> #endif
> ```
> Today, if `FOO` is defined to a nonzero value, you'll get diagnostics for all 
> of those, but if `FOO` is not defined or is defined to 0, then there's no 
> diagnostics. I think we want to consider directives that are *very likely* to 
> be a typo (edit distance of 1, maybe 2?) for a conditional directive as a 
> special case.
> 
> Currently, we only diagnose unknown directives as an error. I think for these 
> special cased conditional directive diagnostics, we'll want to use a warning 
> rather than an error in this circumstance (just in case it turns out to be a 
> valid directive in a skipped conditional block). If we do go that route and 
> make it a warning, I think the warning group should be `-Wunknown-directives` 
> to mirror `-Wunknown-pragmas`, `-Wunknown-attributes`, etc and it should be 
> defined to have the same text as the error case. e.g., 
> ```
> def err_pp_invalid_directive : Error<
>   "invalid preprocessing directive%select{|; did you mean '#%1'?}0"
> >;
> def warn_pp_invalid_directive : Warning<
>   err_pp_invalid_directive.Text>, InGroup>;
> ```
> WDYT?
> 
> (These were my thoughts before seeing the rest of the patch. After reading 
> the patch, it looks like we have pretty similar ideas here, which is great, 
> but leaving the comment anyway in case you have further opinions.)
> Currently, we only diagnose unknown directives as an error. I think for these 
> special cased conditional directive diagnostics, we'll want to use a warning 
> rather than an error in this circumstance (just in case it turns out to be a 
> valid directive in a skipped conditional block). If we do go that route and 
> make it a warning, I think the warning group should be `-Wunknown-directives` 
> to mirror `-Wunknown-pragmas`, `-Wunknown-attributes`, etc and it should be 
> defined to have the same text as the error case. e.g., 
> ```
> def err_pp_invalid_directive : Error<
>   "invalid preprocessing directive%select{|; did you mean '#%1'?}0"
> >;
> def warn_pp_invalid_directive : Warning<
>   err_pp_invalid_directive.Text>, InGroup>;
> ```
> WDYT?
> 
> (These were my thoughts before seeing the rest of the patch. After reading 
> the patch, it looks like we have pretty similar ideas here, which is great, 
> but leaving the comment anyway in case you have further opinions.)

For now, I totally agree with deriving a new warning from 
`err_pp_invalid_directive`.

However, for future scalability, I think it would be great if we could split 
those diagnostics into Error & Warning and Help, for example. Rustc does split 
the diagnostics like the following, and I think this is quite clear. So, a bit 
apart from this patch, I speculate creating a diagnostic system that can split 
them would bring Clang diagnostics much more readable.

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@aaron.ballman

> If so, I think putting Diag after the call of this function would be better.

With the above change, I tried to add comments to failed tests, but there were 
over 300 files.

During my investigation, I found most tests printed warnings without file 
information strangely.

  error: 'warning' diagnostics seen but not expected: 
Line 0: this style of line directive is a GNU extension
Line 0: this style of line directive is a GNU extension
  2 errors generated.

If warnings have file information, it should be like:

  error: 'warning' diagnostics seen but not expected: 
File /tmp/llvm-project/clang/test/Preprocessor/line-directive.c Line 9: 
this style of line directive is a GNU extension
  1 error generated.

Even tests that completely do not use preprocessor directives, such as 
`clang/test/SemaCXX/matrix-type.cpp`, failed with the above strange warnings.

Thus, I suspect that the warnings without file paths have come from internally 
included SDK (I'm using macOS that includes 
`/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk`)
 or something that is not related to target test files.

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:1356
+
+PP.Diag(FlagTok, diag::ext_pp_gnu_line_directive);
   } else if (FlagVal == 2) {

aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > ken-matsui wrote:
> > > > aaron.ballman wrote:
> > > > > I speculate that this change is wrong.
> > > > > 
> > > > > The goal here is to diagnose any time there's a GNU line marker and 
> > > > > now we're only trigging the diagnostic when the line marker's value 
> > > > > is 1; that misses diagnostics when the marker value is something else.
> > > > > 
> > > > > That's why I suggested warning each place we return `false` from this 
> > > > > function -- those are the situations when the line marker is 
> > > > > syntactically correct and we're going to make use of it in the 
> > > > > caller. (We don't want to warn about use of a line marker when we're 
> > > > > going to generate an error anyway.)
> > > > @aaron.ballman 
> > > > 
> > > > Thank you!
> > > > 
> > > > Just to confirm, do I need to remove the call of `Diag` after 
> > > > `GetLineValue` and put `Diag`s into all branches of returning `false` 
> > > > in this function?
> > > > If so, I think putting `Diag` after the call of this function would be 
> > > > better.
> > > > If so, I think putting Diag after the call of this function would be 
> > > > better.
> > > 
> > > You are correct and I agree, good suggestion!
> > @aaron.ballman 
> > Thank you for your response!
> > 
> > I've updated the code as mentioned, but a bunch of other tests with the 
> > `-pedantic` option failed as the following warnings:
> > 
> > ```
> >  TEST 'Clang :: CXX/expr/expr.const/p2-0x.cpp' FAILED 
> > 
> > Script:
> > --
> > : 'RUN: at line 1';   /tmp/llvm/llvm-project/build/bin/clang -cc1 
> > -internal-isystem /tmp/llvm/llvm-project/build/lib/clang/15.0.0/include 
> > -nostdsysteminc -fsyntax-only -std=c++11 -pedantic -verify=expected,cxx11 
> > -fcxx-exceptions 
> > /tmp/llvm/llvm-project/clang/test/CXX/expr/expr.const/p2-0x.cpp 
> > -fconstexpr-depth 128 -triple i686-pc-linux-gnu
> > : 'RUN: at line 2';   /tmp/llvm/llvm-project/build/bin/clang -cc1 
> > -internal-isystem /tmp/llvm/llvm-project/build/lib/clang/15.0.0/include 
> > -nostdsysteminc -fsyntax-only -std=c++2a -pedantic -verify=expected,cxx20 
> > -fcxx-exceptions 
> > /tmp/llvm/llvm-project/clang/test/CXX/expr/expr.const/p2-0x.cpp 
> > -fconstexpr-depth 128 -triple i686-pc-linux-gnu
> > --
> > Exit Code: 1
> > 
> > Command Output (stderr):
> > --
> > error: 'warning' diagnostics seen but not expected: 
> >   Line 0: this style of line directive is a GNU extension
> >   Line 0: this style of line directive is a GNU extension
> > 2 errors generated.
> > 
> > ...
> > ```
> > 
> > I personally think it would be preferable if the only change of tests would 
> > be `line-directive.c`.
> > So, how about reducing `Diag` calls until the warning doesn't spill over 
> > into other tests?
> > I personally think it would be preferable if the only change of tests would 
> > be line-directive.c.
> > So, how about reducing Diag calls until the warning doesn't spill over into 
> > other tests?
> 
> No, this is expected. We're adding a diagnostic where there wasn't one 
> previously, so some files are going to get caught by that. You can either add 
> the `// expected-warning {{}}` comments to those lines, or if the test has a 
> lot of those lines but isn't really specific to line markers (it just happens 
> to use them to test other functionality) you can disable the diagnostic for 
> that test with `-Wno-gnu-line-marker`.
Ah, I see. I'm going to work on it. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/test/OpenMP/predefined_macro.c:7
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -verify -o - %s
-// expected-no-diagnostics
+// expected-warning@+5 {{'#elsif' directive not found, did you mean '#elif'?}}
 #ifdef FOPENMP

I am not sure if this typo was intended.

When I renamed `elsif` to `elif`, `#error "_OPENMP has incorrect value"` on 
line `13` was evaluated.

Therefore, if this typo was intended, just suppressing the warning with 
`expected-warning` would be better. However, if this typo was NOT intended, I 
think I should make `_OPENMP` equal to `201107`. It is also possible that this 
test was mistakenly written.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui created this revision.
ken-matsui added a reviewer: aaron.ballman.
Herald added a subscriber: mgorny.
Herald added a project: All.
ken-matsui requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch implements suggestion for typoed directives in preprocessor 
conditionals. The related issue is: 
https://github.com/llvm/llvm-project/issues/51598.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Tooling/LevDistance.h
  clang/lib/Lex/CMakeLists.txt
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/LevDistance.cpp
  clang/test/OpenMP/predefined_macro.c
  clang/test/Preprocessor/suggest-typoed-directive.c
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/LevDistanceTest.cpp

Index: clang/unittests/Tooling/LevDistanceTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/LevDistanceTest.cpp
@@ -0,0 +1,60 @@
+//===- unittest/Tooling/LevDistanceTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/LevDistance.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace clang;
+
+using tooling::levdistance::findSimilarStr;
+using tooling::levdistance::levDistance;
+
+namespace {
+
+TEST(LevDistanceTest, levDistance) {
+  EXPECT_EQ(0ul, levDistance("", ""));
+  EXPECT_EQ(1ul, levDistance("", "aaab"));
+  EXPECT_EQ(2ul, levDistance("aabc", "aacb"));
+  EXPECT_EQ(2ul, levDistance("aabc", "abca"));
+  EXPECT_EQ(3ul, levDistance("aabc", "adef"));
+  EXPECT_EQ(4ul, levDistance("abcd", "efgh"));
+}
+
+TEST(LevDistanceTest, findSimilarStr) {
+  std::array candidates{"aaab", "aaabc"};
+  EXPECT_EQ(std::string("aaab"), findSimilarStr(candidates, ""));
+
+  candidates = {"aab", "abc"};
+  EXPECT_EQ(std::string("aab"), findSimilarStr(candidates, "aaa"));
+
+  candidates = {"ab", "bc"};
+  EXPECT_EQ(std::string("ab"), findSimilarStr(candidates, "aa"));
+
+  candidates = {"b", "c"};
+  EXPECT_EQ(None, findSimilarStr(candidates, "a"));
+}
+
+TEST(LevDistanceTest, findSimilarStrToMacros) {
+  const std::array candidates{
+  "if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"};
+
+  EXPECT_EQ(std::string("elifdef"), findSimilarStr(candidates, "elfidef"));
+  EXPECT_EQ(std::string("elifdef"), findSimilarStr(candidates, "elifdef"));
+  EXPECT_EQ(None, findSimilarStr(candidates, "special_compiler_directive"));
+}
+
+TEST(LevDistanceTest, findSimilarStrInCaseInsensitive) {
+  const std::array candidates{
+  "if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"};
+
+  EXPECT_EQ(std::string("elifdef"), findSimilarStr(candidates, "elifdef"));
+  EXPECT_EQ(std::string("elifdef"), findSimilarStr(candidates, "ELIFDEF"));
+}
+
+} // end anonymous namespace
Index: clang/unittests/Tooling/CMakeLists.txt
===
--- clang/unittests/Tooling/CMakeLists.txt
+++ clang/unittests/Tooling/CMakeLists.txt
@@ -18,6 +18,7 @@
   FixItTest.cpp
   HeaderIncludesTest.cpp
   StandardLibraryTest.cpp
+  LevDistanceTest.cpp
   LexicallyOrderedRecursiveASTVisitorTest.cpp
   LookupTest.cpp
   QualTypeNamesTest.cpp
Index: clang/test/Preprocessor/suggest-typoed-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -pedantic %s
+
+// expected-warning@+11 {{'#id' directive not found, did you mean '#if'?}}
+// expected-warning@+11 {{'#ifd' directive not found, did you mean '#if'?}}
+// expected-warning@+11 {{'#ifde' directive not found, did you mean '#ifdef'?}}
+// expected-warning@+11 {{'#elid' directive not found, did you mean '#elif'?}}
+// expected-warning@+11 {{'#elsif' directive not found, did you mean '#elif'?}}
+// expected-warning@+11 {{'#elseif' directive not found, did you mean '#elif'?}}
+// expected-warning@+11 {{'#elfidef' directive not found, did you mean '#elifdef'?}}
+// expected-warning@+11 {{'#elfindef' directive not found, did you mean '#elifdef'?}}
+// expected-warning@+11 {{'#elsi' directive not found, did you mean '#else'?}}
+// expected-warning@+11 {{'#endi' directive not found, did you mean '#endif'?}}
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elid
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elsi
+#endi
+#endif
+
+#if special_compiler
+#special_compiler_directive // no diagnostics
+#endif
+

[PATCH] D124534: Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 426185.
ken-matsui added a comment.

Add a diagnostic for line directive of a gnu extension


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/line-directive.c

Index: clang/test/Preprocessor/line-directive.c
===
--- clang/test/Preprocessor/line-directive.c
+++ clang/test/Preprocessor/line-directive.c
@@ -27,8 +27,8 @@
 #define A 42 "foo"
 #line A
 
-# 42
-# 42 "foo"
+# 42 // expected-warning {{this style of line directive is a GNU extension}}
+# 42 "foo" // expected-warning {{this style of line directive is a GNU extension}}
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
 # 42 "foo" 1 3  // enter
 # 42 "foo" 2 3  // exit
@@ -97,7 +97,7 @@
 #line 010  // expected-warning {{#line directive interprets number as decimal, not octal}}
 extern int array[__LINE__ == 10 ? 1:-1];
 
-# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}}
+# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}} expected-warning {{this style of line directive is a GNU extension}}
 extern int array_gnuline[__LINE__ == 20 ? 1:-1];
 
 /* PR3917 */
@@ -106,7 +106,7 @@
 _\
 _LINE__ == 42 ? 1: -1];  /* line marker is location of first _ */
 
-# 51
+# 51 // expected-warning {{this style of line directive is a GNU extension}}
 extern char array2_gnuline[\
 _\
 _LINE__ == 52 ? 1: -1];  /* line marker is location of first _ */
@@ -115,7 +115,7 @@
 #line 0 "line-directive.c" // expected-warning {{#line directive with zero argument is a GNU extension}}
 undefined t; // expected-error {{unknown type name 'undefined'}}
 
-# 115 "main"
+# 115 "main" // expected-warning {{this style of line directive is a GNU extension}}
 # 116 "enter-1" 1
 # 117 "enter-2" 1
 # 118 "" 2 // pop to enter-1
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1335,7 +1335,10 @@
   unsigned FlagVal;
   Token FlagTok;
   PP.Lex(FlagTok);
-  if (FlagTok.is(tok::eod)) return false;
+  if (FlagTok.is(tok::eod)) {
+PP.Diag(FlagTok, diag::ext_pp_gnu_line_directive);
+return false;
+  }
   if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag, PP))
 return true;
 
@@ -1431,6 +1434,7 @@
   // If the StrTok is "eod", then it wasn't present.  Otherwise, it must be a
   // string followed by eod.
   if (StrTok.is(tok::eod)) {
+Diag(StrTok, diag::ext_pp_gnu_line_directive);
 // Treat this like "#line NN", which doesn't change file characteristics.
 FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());
   } else if (StrTok.isNot(tok::string_literal)) {
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -427,6 +427,10 @@
 def ext_pp_opencl_variadic_macros : Extension<
   "variadic macros are a Clang extension in OpenCL">;
 
+def ext_pp_gnu_line_directive : Extension<
+  "this style of line directive is a GNU extension">,
+  InGroup;
+
 def err_pp_invalid_directive : Error<"invalid preprocessing directive">;
 def err_pp_directive_required : Error<
   "%0 must be used within a preprocessing directive">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -450,6 +450,7 @@
 def : DiagGroup<"inline">;
 def : DiagGroup<"invalid-pch">;
 def GNULabelsAsValue : DiagGroup<"gnu-label-as-value">;
+def GNULineMarker : DiagGroup<"gnu-line-marker">;
 def LiteralRange : DiagGroup<"literal-range">;
 def LocalTypeTemplateArgs : DiagGroup<"local-type-template-args",
   [CXX98CompatLocalTypeTemplateArgs]>;
@@ -,7 +1112,7 @@
 VLAExtension, GNUFlexibleArrayInitializer,
 GNUFlexibleArrayUnionMember, GNUFoldingConstant,
 GNUImaginaryConstant, GNUIncludeNext,
-GNULabelsAsValue, GNUNullPointerArithmetic,
+GNULabelsAsValue, GNULineMarker, GNUNullPointerArithmetic,
 GNUPointerArith, RedeclaredClassMember,
 GNURedeclaredEnum, GNUStatementExpression,
 GNUStaticFloatInit,
___
cfe-commits mailing 

[PATCH] D124534: Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:1356
+
+PP.Diag(FlagTok, diag::ext_pp_gnu_line_directive);
   } else if (FlagVal == 2) {

aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > I speculate that this change is wrong.
> > > 
> > > The goal here is to diagnose any time there's a GNU line marker and now 
> > > we're only trigging the diagnostic when the line marker's value is 1; 
> > > that misses diagnostics when the marker value is something else.
> > > 
> > > That's why I suggested warning each place we return `false` from this 
> > > function -- those are the situations when the line marker is 
> > > syntactically correct and we're going to make use of it in the caller. 
> > > (We don't want to warn about use of a line marker when we're going to 
> > > generate an error anyway.)
> > @aaron.ballman 
> > 
> > Thank you!
> > 
> > Just to confirm, do I need to remove the call of `Diag` after 
> > `GetLineValue` and put `Diag`s into all branches of returning `false` in 
> > this function?
> > If so, I think putting `Diag` after the call of this function would be 
> > better.
> > If so, I think putting Diag after the call of this function would be better.
> 
> You are correct and I agree, good suggestion!
@aaron.ballman 
Thank you for your response!

I've updated the code as mentioned, but a bunch of other tests with the 
`-pedantic` option failed as the following warnings:

```
 TEST 'Clang :: CXX/expr/expr.const/p2-0x.cpp' FAILED 

Script:
--
: 'RUN: at line 1';   /tmp/llvm/llvm-project/build/bin/clang -cc1 
-internal-isystem /tmp/llvm/llvm-project/build/lib/clang/15.0.0/include 
-nostdsysteminc -fsyntax-only -std=c++11 -pedantic -verify=expected,cxx11 
-fcxx-exceptions 
/tmp/llvm/llvm-project/clang/test/CXX/expr/expr.const/p2-0x.cpp 
-fconstexpr-depth 128 -triple i686-pc-linux-gnu
: 'RUN: at line 2';   /tmp/llvm/llvm-project/build/bin/clang -cc1 
-internal-isystem /tmp/llvm/llvm-project/build/lib/clang/15.0.0/include 
-nostdsysteminc -fsyntax-only -std=c++2a -pedantic -verify=expected,cxx20 
-fcxx-exceptions 
/tmp/llvm/llvm-project/clang/test/CXX/expr/expr.const/p2-0x.cpp 
-fconstexpr-depth 128 -triple i686-pc-linux-gnu
--
Exit Code: 1

Command Output (stderr):
--
error: 'warning' diagnostics seen but not expected: 
  Line 0: this style of line directive is a GNU extension
  Line 0: this style of line directive is a GNU extension
2 errors generated.

...
```

I personally think it would be preferable if the only change of tests would be 
`line-directive.c`.
So, how about reducing `Diag` calls until the warning doesn't spill over into 
other tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124534: Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:1356
+
+PP.Diag(FlagTok, diag::ext_pp_gnu_line_directive);
   } else if (FlagVal == 2) {

aaron.ballman wrote:
> I speculate that this change is wrong.
> 
> The goal here is to diagnose any time there's a GNU line marker and now we're 
> only trigging the diagnostic when the line marker's value is 1; that misses 
> diagnostics when the marker value is something else.
> 
> That's why I suggested warning each place we return `false` from this 
> function -- those are the situations when the line marker is syntactically 
> correct and we're going to make use of it in the caller. (We don't want to 
> warn about use of a line marker when we're going to generate an error anyway.)
@aaron.ballman 

Thank you!

Just to confirm, do I need to remove the call of `Diag` after `GetLineValue` 
and put `Diag`s into all branches of returning `false` in this function?
If so, I think putting `Diag` after the call of this function would be better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124556: [NFC] Prevent shadowing a variable declared in `if`

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124556

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

[PATCH] D124534: Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Hi, @aaron.ballman

Thanks for your kind reviews!
I’ve updated the code, but is this what you’d expected?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124534: Add a diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 425916.
ken-matsui added a comment.

Add a diagnostic for line directive of a gnu extension


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Misc/warning-flags.c
  clang/test/Preprocessor/line-directive.c


Index: clang/test/Preprocessor/line-directive.c
===
--- clang/test/Preprocessor/line-directive.c
+++ clang/test/Preprocessor/line-directive.c
@@ -9,7 +9,7 @@
 # 20 "" 2
 
 // a push/pop before any other line control
-# 10 "enter-0" 1
+# 10 "enter-0" 1 // expected-warning {{this style of line directive is a GNU 
extension}}
 # 11 "" 2 // pop to main file
 #error MAIN7
 // expected-error@-1{{MAIN7}}
@@ -30,7 +30,7 @@
 # 42 // expected-warning {{this style of line directive is a GNU extension}}
 # 42 "foo" // expected-warning {{this style of line directive is a GNU 
extension}}
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop 
empty include stack}}
-# 42 "foo" 1 3  // enter
+# 42 "foo" 1 3  // enter expected-warning {{this style of line directive is a 
GNU extension}}
 # 42 "foo" 2 3  // exit
 # 42 "foo" 2 3 4 // expected-error {{invalid line marker flag '2': cannot pop 
empty include stack}}
 # 42 "foo" 3 4
@@ -116,8 +116,8 @@
 undefined t; // expected-error {{unknown type name 'undefined'}}
 
 # 115 "main" // expected-warning {{this style of line directive is a GNU 
extension}}
-# 116 "enter-1" 1
-# 117 "enter-2" 1
+# 116 "enter-1" 1 // expected-warning {{this style of line directive is a GNU 
extension}}
+# 117 "enter-2" 1 // expected-warning {{this style of line directive is a GNU 
extension}}
 # 118 "" 2 // pop to enter-1
 #error ENTER1
 // expected-error@-1{{ENTER1}}
Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -90,4 +90,4 @@
 
 The list of warnings in -Wpedantic should NEVER grow.
 
-CHECK: Number in -Wpedantic (not covered by other -W flags): 28
+CHECK: Number in -Wpedantic (not covered by other -W flags): 27
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1346,9 +1346,14 @@
 IsFileEntry = true;
 
 PP.Lex(FlagTok);
-if (FlagTok.is(tok::eod)) return false;
+if (FlagTok.is(tok::eod)) {
+  PP.Diag(FlagTok, diag::ext_pp_gnu_line_directive);
+  return false;
+}
 if (GetLineValue(FlagTok, FlagVal, 
diag::err_pp_linemarker_invalid_flag,PP))
   return true;
+
+PP.Diag(FlagTok, diag::ext_pp_gnu_line_directive);
   } else if (FlagVal == 2) {
 IsFileExit = true;
 
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -428,7 +428,8 @@
   "variadic macros are a Clang extension in OpenCL">;
 
 def ext_pp_gnu_line_directive : Extension<
-  "this style of line directive is a GNU extension">;
+  "this style of line directive is a GNU extension">,
+  InGroup;
 
 def err_pp_invalid_directive : Error<"invalid preprocessing directive">;
 def err_pp_directive_required : Error<
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -450,6 +450,7 @@
 def : DiagGroup<"inline">;
 def : DiagGroup<"invalid-pch">;
 def GNULabelsAsValue : DiagGroup<"gnu-label-as-value">;
+def GNULineMarker : DiagGroup<"gnu-line-marker">;
 def LiteralRange : DiagGroup<"literal-range">;
 def LocalTypeTemplateArgs : DiagGroup<"local-type-template-args",
   [CXX98CompatLocalTypeTemplateArgs]>;


Index: clang/test/Preprocessor/line-directive.c
===
--- clang/test/Preprocessor/line-directive.c
+++ clang/test/Preprocessor/line-directive.c
@@ -9,7 +9,7 @@
 # 20 "" 2
 
 // a push/pop before any other line control
-# 10 "enter-0" 1
+# 10 "enter-0" 1 // expected-warning {{this style of line directive is a GNU extension}}
 # 11 "" 2 // pop to main file
 #error MAIN7
 // expected-error@-1{{MAIN7}}
@@ -30,7 +30,7 @@
 # 42 // expected-warning {{this style of line directive is a GNU extension}}
 # 42 "foo" // expected-warning {{this style of line directive is a GNU extension}}
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
-# 42 "foo" 1 3  // enter
+# 42 "foo" 1 3  // enter expected-warning {{this 

[PATCH] D124556: [NFC] Prevent shadowing a variable declared in `if`

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@hubert.reinterpretcast

I see. Thank you for the detailed information!

Could you please commit this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124556

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

[PATCH] D124556: [NFC] Prevent shadowing a variable declared in `if`

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@hubert.reinterpretcast

Sorry, I'm a newbie here, but is there anything I should do after getting 
approved?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124556

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

[PATCH] D124556: [NFC] Prevent shadowing a variable declared in `if`

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you for your review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124556

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

[PATCH] D124556: [NFC] Prevent shadowing a variable declared in `if`

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@hubert.reinterpretcast,

Thank you for your suggestion! I’ve updated the summary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124556

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

[PATCH] D124556: [NFC] Prevent shadowing a variable declared in `if`

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@hubert.reinterpretcast,

Sorry to have missed providing a summary.

In most cases, shadowing a declaration of a local variable should be avoided to 
prevent making others confused and mistakes because we need to figure out the 
boundaries of the declaration. In this case, the variable declared by `const 
char *S` in `if` can be accessed by every branch. That means that declarations 
in `else if` had shadowed the declaration in `if`, so I changed them to use the 
same declaration used in `if`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124556

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

[PATCH] D124556: Prevent shadowing a variable declared in `if`

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui created this revision.
Herald added a project: All.
ken-matsui requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124556

Files:
  clang/lib/Basic/Diagnostic.cpp


Index: clang/lib/Basic/Diagnostic.cpp
===
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -983,13 +983,13 @@
   if (const char *S = tok::getPunctuatorSpelling(Kind))
 // Quoted token spelling for punctuators.
 Out << '\'' << S << '\'';
-  else if (const char *S = tok::getKeywordSpelling(Kind))
+  else if ((S = tok::getKeywordSpelling(Kind)))
 // Unquoted token spelling for keywords.
 Out << S;
-  else if (const char *S = getTokenDescForDiagnostic(Kind))
+  else if ((S = getTokenDescForDiagnostic(Kind)))
 // Unquoted translatable token name.
 Out << S;
-  else if (const char *S = tok::getTokenName(Kind))
+  else if ((S = tok::getTokenName(Kind)))
 // Debug name, shouldn't appear in user-facing diagnostics.
 Out << '<' << S << '>';
   else


Index: clang/lib/Basic/Diagnostic.cpp
===
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -983,13 +983,13 @@
   if (const char *S = tok::getPunctuatorSpelling(Kind))
 // Quoted token spelling for punctuators.
 Out << '\'' << S << '\'';
-  else if (const char *S = tok::getKeywordSpelling(Kind))
+  else if ((S = tok::getKeywordSpelling(Kind)))
 // Unquoted token spelling for keywords.
 Out << S;
-  else if (const char *S = getTokenDescForDiagnostic(Kind))
+  else if ((S = getTokenDescForDiagnostic(Kind)))
 // Unquoted translatable token name.
 Out << S;
-  else if (const char *S = tok::getTokenName(Kind))
+  else if ((S = tok::getTokenName(Kind)))
 // Debug name, shouldn't appear in user-facing diagnostics.
 Out << '<' << S << '>';
   else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124534: Add a warning diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Hi @aaron.ballman,

Thank you for your review! I updated the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124534: Add a warning diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 425601.
ken-matsui added a comment.

Add a diagnostic for line directive of a gnu extension


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Misc/warning-flags.c
  clang/test/Preprocessor/line-directive.c


Index: clang/test/Preprocessor/line-directive.c
===
--- clang/test/Preprocessor/line-directive.c
+++ clang/test/Preprocessor/line-directive.c
@@ -27,8 +27,8 @@
 #define A 42 "foo"
 #line A
 
-# 42
-# 42 "foo"
+# 42 // expected-warning {{this style of line directive is a GNU extension}}
+# 42 "foo" // expected-warning {{this style of line directive is a GNU 
extension}}
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop 
empty include stack}}
 # 42 "foo" 1 3  // enter
 # 42 "foo" 2 3  // exit
@@ -97,7 +97,7 @@
 #line 010  // expected-warning {{#line directive interprets number as decimal, 
not octal}}
 extern int array[__LINE__ == 10 ? 1:-1];
 
-# 020  // expected-warning {{GNU line marker directive interprets number 
as decimal, not octal}}
+# 020  // expected-warning {{GNU line marker directive interprets number 
as decimal, not octal}} expected-warning {{this style of line directive is a 
GNU extension}}
 extern int array_gnuline[__LINE__ == 20 ? 1:-1];
 
 /* PR3917 */
@@ -106,7 +106,7 @@
 _\
 _LINE__ == 42 ? 1: -1];  /* line marker is location of first _ */
 
-# 51
+# 51 // expected-warning {{this style of line directive is a GNU extension}}
 extern char array2_gnuline[\
 _\
 _LINE__ == 52 ? 1: -1];  /* line marker is location of first _ */
@@ -115,7 +115,7 @@
 #line 0 "line-directive.c" // expected-warning {{#line directive with zero 
argument is a GNU extension}}
 undefined t; // expected-error {{unknown type name 'undefined'}}
 
-# 115 "main"
+# 115 "main" // expected-warning {{this style of line directive is a GNU 
extension}}
 # 116 "enter-1" 1
 # 117 "enter-2" 1
 # 118 "" 2 // pop to enter-1
Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -90,4 +90,4 @@
 
 The list of warnings in -Wpedantic should NEVER grow.
 
-CHECK: Number in -Wpedantic (not covered by other -W flags): 27
+CHECK: Number in -Wpedantic (not covered by other -W flags): 28
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1335,7 +1335,10 @@
   unsigned FlagVal;
   Token FlagTok;
   PP.Lex(FlagTok);
-  if (FlagTok.is(tok::eod)) return false;
+  if (FlagTok.is(tok::eod)) {
+PP.Diag(FlagTok, diag::ext_pp_gnu_line_directive);
+return false;
+  }
   if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag, PP))
 return true;
 
@@ -1431,6 +1434,7 @@
   // If the StrTok is "eod", then it wasn't present.  Otherwise, it must be a
   // string followed by eod.
   if (StrTok.is(tok::eod)) {
+Diag(StrTok, diag::ext_pp_gnu_line_directive);
 // Treat this like "#line NN", which doesn't change file characteristics.
 FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());
   } else if (StrTok.isNot(tok::string_literal)) {
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -427,6 +427,9 @@
 def ext_pp_opencl_variadic_macros : Extension<
   "variadic macros are a Clang extension in OpenCL">;
 
+def ext_pp_gnu_line_directive : Extension<
+  "this style of line directive is a GNU extension">;
+
 def err_pp_invalid_directive : Error<"invalid preprocessing directive">;
 def err_pp_directive_required : Error<
   "%0 must be used within a preprocessing directive">;


Index: clang/test/Preprocessor/line-directive.c
===
--- clang/test/Preprocessor/line-directive.c
+++ clang/test/Preprocessor/line-directive.c
@@ -27,8 +27,8 @@
 #define A 42 "foo"
 #line A
 
-# 42
-# 42 "foo"
+# 42 // expected-warning {{this style of line directive is a GNU extension}}
+# 42 "foo" // expected-warning {{this style of line directive is a GNU extension}}
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
 # 42 "foo" 1 3  // enter
 # 42 "foo" 2 3  // exit
@@ -97,7 +97,7 @@
 #line 010  // expected-warning {{#line directive interprets number as decimal, not octal}}
 extern int array[__LINE__ == 10 ? 1:-1];
 
-# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}}
+# 020  // expected-warning {{GNU line marker 

[PATCH] D124534: Add a warning diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

This change is related to: https://github.com/llvm/llvm-project/issues/55067


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124534: Add a warning diagnostic for line directive of a gnu extension

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui created this revision.
Herald added a project: All.
ken-matsui requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124534

Files:
  clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
  clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Misc/warning-flags.c
  clang/test/Preprocessor/line-directive.c
  clang/test/Preprocessor/warn-gnu-ext-line-directive.c

Index: clang/test/Preprocessor/warn-gnu-ext-line-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/warn-gnu-ext-line-directive.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+#1 // expected-warning {{style of line directive is a GNU extension}}
Index: clang/test/Preprocessor/line-directive.c
===
--- clang/test/Preprocessor/line-directive.c
+++ clang/test/Preprocessor/line-directive.c
@@ -27,7 +27,7 @@
 #define A 42 "foo"
 #line A
 
-# 42
+# 42 // expected-warning {{style of line directive is a GNU extension}}
 # 42 "foo"
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
 # 42 "foo" 1 3  // enter
@@ -97,7 +97,7 @@
 #line 010  // expected-warning {{#line directive interprets number as decimal, not octal}}
 extern int array[__LINE__ == 10 ? 1:-1];
 
-# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}}
+# 020  // expected-warning {{GNU line marker directive interprets number as decimal, not octal}} expected-warning {{style of line directive is a GNU extension}}
 extern int array_gnuline[__LINE__ == 20 ? 1:-1];
 
 /* PR3917 */
@@ -106,7 +106,7 @@
 _\
 _LINE__ == 42 ? 1: -1];  /* line marker is location of first _ */
 
-# 51
+# 51 // expected-warning {{style of line directive is a GNU extension}}
 extern char array2_gnuline[\
 _\
 _LINE__ == 52 ? 1: -1];  /* line marker is location of first _ */
Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (67):
+CHECK: Warnings without flags (68):
 
 CHECK-NEXT:   ext_expected_semi_decl_list
 CHECK-NEXT:   ext_explicit_specialization_storage_class
@@ -71,6 +71,7 @@
 CHECK-NEXT:   warn_on_superclass_use
 CHECK-NEXT:   warn_pp_convert_to_positive
 CHECK-NEXT:   warn_pp_expr_overflow
+CHECK-NEXT:   warn_pp_gnu_ext_line_directive
 CHECK-NEXT:   warn_pp_line_decimal
 CHECK-NEXT:   warn_pragma_pack_pop_identifier_and_alignment
 CHECK-NEXT:   warn_pragma_pack_show
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1432,6 +1432,7 @@
   // string followed by eod.
   if (StrTok.is(tok::eod)) {
 // Treat this like "#line NN", which doesn't change file characteristics.
+Diag(StrTok, diag::warn_pp_gnu_ext_line_directive);
 FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());
   } else if (StrTok.isNot(tok::string_literal)) {
 Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -427,6 +427,8 @@
 def ext_pp_opencl_variadic_macros : Extension<
   "variadic macros are a Clang extension in OpenCL">;
 
+def warn_pp_gnu_ext_line_directive : Warning<"style of line directive is a GNU extension">;
+
 def err_pp_invalid_directive : Error<"invalid preprocessing directive">;
 def err_pp_directive_required : Error<
   "%0 must be used within a preprocessing directive">;
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
===
--- clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
@@ -1,6 +1,6 @@
-
-// This file intentionally uses a CRLF newlines!
-
-void foo() {
-  int *x = nullptr;
-}
+
+// This file intentionally uses a CRLF newlines!
+
+void foo() {
+  int *x = nullptr;
+}
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
===
--- clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
@@ -1,6