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

[ken-matsui]

Summary

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.

[nickdesaulniers]

clang/test/Preprocessor/suggest-typoed-directive.S

2

Consider adding a test:

// RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp < %s

I don't think you need -S here or -o %t since there should be nothing produced from -fsyntax-only.

[nickdesaulniers]

clang/lib/Lex/PPDirectives.cpp

484

Also, IIRC the token used to start a comment in assembler differs per architecture. This might be the simplest fix, for now.

clang/test/Preprocessor/suggest-typoed-directive.S

2

Sorry, I missed a - for "read from stdin":

// RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp - < %s

[ken-matsui]

Update the code as reviewed

[ken-matsui]

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

[ken-matsui]

clang/lib/Lex/PPDirectives.cpp

484

Ah, I did not know that. Thank you!

[nickdesaulniers]

clang/test/Preprocessor/suggest-typoed-directive.S

2

Sorry, I meant for you to have two run lines. One that specified -x assembler-with-cpp and one without. But I think -x assembler-with-cpp is not necessary, you should/can/may drop it.

[ken-matsui]

Remove -x assembler-with-cpp

[ken-matsui]

Removed it 👍

[nickdesaulniers]

clang/test/Preprocessor/suggest-typoed-directive.S

2

These three can be removed now as well. Clang will read %s as input without the need to read from stdin, which is what - < [filename] is doing.

[ken-matsui]

Remove - < %s

[ken-matsui]

Sorry for having missed it. Thank you!

[nickdesaulniers]

thanks for the patch!

[ken-matsui]

@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: [e-mail]

[nickdesaulniers]

I do not have permission to land this patch, so could you please do it on my behalf?

Will do.

[nickdesaulniers]

Sorry, it looks like arcanist or my PHP runtime was auto updated on my host and has regressed. I need to sort that out first.

[nickdesaulniers]

sorted, will commit soon

[nickdesaulniers]

pushed 45e01ce5fe6a5e4dc25ffdf626caa344fbcb93dd

[ken-matsui]

Thank you!

[aaron.ballman]

I know it's retroactive, but this also LGTM. Thank you for the fix!

clang/lib/Lex/PPDirectives.cpp

Optional<unsigned> Preprocessor::getSkippedRangeForExcludedConditionalBlock(
   unsigned LengthDiff = CurLexerBufferOffset - HashFileOffset.second;
   assert(BytesToSkip >= LengthDiff && "lexer is after the skipped range?");
   return BytesToSkip - LengthDiff;
 }
 
 void Preprocessor::SuggestTypoedDirective(const Token &Tok,
                                           StringRef Directive,
                                           const SourceLocation &EndLoc) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector<StringRef> Candidates = {
       "if", "ifdef", "ifndef", "elif", "else", "endif"
   };
   if (LangOpts.C2x || LangOpts.CPlusPlus2b)
     Candidates.insert(Candidates.end(), {"elifdef", "elifndef"});
 
   if (Optional<StringRef> Sugg = findSimilarStr(Directive, Candidates)) {
     CharSourceRange DirectiveRange =

clang/test/Preprocessor/suggest-typoed-directive.S

+// 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