[PATCH] D31496: Make -defsym a driver option

[Salman Arif via Phabricator via cfe-commits]

salari01 created this revision.

Extended the integrated assembler -Wa,-defsym option to be usable with the 
Clang driver. Both options arehandled in the same way. Using -defsym when 
not assembling files shows an unused argument warning.


https://reviews.llvm.org/D31496

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/defsym.c
  test/Driver/defsym.s

Index: test/Driver/defsym.s
===
--- test/Driver/defsym.s
+++ test/Driver/defsym.s
@@ -1,37 +1,73 @@
 // RUN: %clang -### -c -integrated-as %s \
 // RUN: -Wa,-defsym,abc=5 -Wa,-defsym,xyz=0xa \
 // RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM1
+// RUN: %clang -### -c -integrated-as %s \
+// RUN: -defsym abc=5 -defsym xyz=0xa \
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM1
 
 // RUN: %clang -### -c -no-integrated-as -target x86_64-unknown-unknown %s \
 // RUN: -Wa,-defsym,abc=5 -Wa,-defsym,xyz=0xa \
 // RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM1
+// RUN: %clang -### -c -no-integrated-as -target x86_64-unknown-unknown %s \
+// RUN: -defsym abc=5 -defsym xyz=0xa \
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-UNUSED
 
 // CHECK-DEFSYM1: "-defsym"
 // CHECK-DEFSYM1: "abc=5"
 // CHECK-DEFSYM1: "-defsym"
 // CHECK-DEFSYM1: "xyz=0xa"
+// CHECK-UNUSED:  warning: argument unused during compilation: '-defsym abc=5' [-Wunused-command-line-argument]
+// CHECK-UNUSED:  warning: argument unused during compilation: '-defsym xyz=0xa' [-Wunused-command-line-argument]
 
 // RUN: not %clang -c -integrated-as -o /dev/null %s \
 // RUN: -Wa,-defsym,abc= \
 // RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR1
+// RUN: not %clang -c -integrated-as -o /dev/null %s \
+// RUN: -defsym abc= \
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR1
 // CHECK-DEFSYM-ERR1: error: defsym must be of the form: sym=value: abc=
 
 // RUN: not %clang -c -integrated-as -o /dev/null %s \
-// RUN: -Wa,-defsym,=123 \
+// RUN: -Wa,-defsym,abc \
 // RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR2
-// CHECK-DEFSYM-ERR2: error: defsym must be of the form: sym=value: =123
+// RUN: not %clang -c -integrated-as -o /dev/null %s \
+// RUN: -defsym abc \
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR2
+// CHECK-DEFSYM-ERR2: error: defsym must be of the form: sym=value: abc
 
 // RUN: not %clang -c -integrated-as -o /dev/null %s \
-// RUN: -Wa,-defsym,abc=1a2b3c \
+// RUN: -Wa,-defsym,=123 \
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR3
+// RUN: not %clang -c -integrated-as -o /dev/null %s \
+// RUN: -defsym =123 \
 // RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR3
-// CHECK-DEFSYM-ERR3: error: Value is not an integer: 1a2b3c
+// CHECK-DEFSYM-ERR3: error: defsym must be of the form: sym=value: =123
 
 // RUN: not %clang -c -integrated-as -o /dev/null %s \
-// RUN: -Wa,-defsym \
+// RUN: -Wa,-defsym,123 \
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR4
+// RUN: not %clang -c -integrated-as -o /dev/null %s \
+// RUN: -defsym 123 \
 // RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR4
+// CHECK-DEFSYM-ERR4: error: defsym must be of the form: sym=value: 123
 
 // RUN: not %clang -c -integrated-as -o /dev/null %s \
+// RUN: -Wa,-defsym,abc=1a2b3c \
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR5
+// RUN: not %clang -c -integrated-as -o /dev/null %s \
+// RUN: -defsym abc=1a2b3c \
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR5
+// CHECK-DEFSYM-ERR5: error: Value is not an integer: 1a2b3c
+
+// RUN: not %clang -c -integrated-as -o /dev/null %s \
+// RUN: -Wa,-defsym \
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR6
+// RUN: not %clang -c -integrated-as -o /dev/null %s \
 // RUN: -Wa,-defsym, \
-// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR4
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR6
+// CHECK-DEFSYM-ERR6: error: defsym must be of the form: sym=value: -defsym
 
-// CHECK-DEFSYM-ERR4: error: defsym must be of the form: sym=value: -defsym
+// RUN: not %clang -c -integrated-as -o /dev/null %s \
+// RUN: -defsym \
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR7
+// CHECK-DEFSYM-ERR7: error: argument to '-defsym' is missing (expected 1 value)
\ No newline at end of file
Index: test/Driver/defsym.c
===
--- /dev/null
+++ test/Driver/defsym.c
@@ -0,0 +1,13 @@
+// RUN: not %clang -c -o /dev/null %s \
+// RUN: -defsym \
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-ERR
+// CHECK-DEFSYM-ERR: error: argument to '-defsym' is missing (expected 1 value)
+
+// RUN: %clang -c -o /dev/null %s \
+// RUN: -defsym bar=1 \
+// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-DEFSYM-WARN
+// CHECK-DEFSYM-WARN: warning: argument unused during compilation: '-defsym bar=1'
+
+int foo

[PATCH] D31496: Make -defsym a driver option

[Salman Arif via Phabricator via cfe-commits]

salari01 added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:1882
+TakeNextArg = true;
   }
   } else {

rogfer01 wrote:
> I wonder if you should `break;` here if validation fails like the original 
> code did?
The reason for removing it is that this way, all erroneous `-defsym` options 
specified can be listed during the same invocation. This provides a better user 
experience than tackling them one at a time, as with the original code.


https://reviews.llvm.org/D31496



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

[PATCH] D31765: Skip Unicode character expansion in assembly files

[Salman Arif via Phabricator via cfe-commits]

salari01 created this revision.

When using the C preprocessor with assembly files, either with a capital `S` 
file extension, or with `-xassembler-with-cpp`, the Unicode escape sequence 
`\u` is ignored. The `\u` pattern can be used for expanding a macro argument 
that starts with `u`.


https://reviews.llvm.org/D31765

Files:
  lib/Lex/Lexer.cpp
  test/Lexer/asm-preproc-no-unicode.s


Index: test/Lexer/asm-preproc-no-unicode.s
===
--- /dev/null
+++ test/Lexer/asm-preproc-no-unicode.s
@@ -0,0 +1,13 @@
+// RUN: %clang --target=arm-arm-none-eabi -c -xassembler-with-cpp %s -o %t 
2>&1 | FileCheck %s --check-prefix=WARNING
+// RUN: llvm-objdump -s %t | FileCheck %s --check-prefix=DATA
+
+// WARNING-NOT: warning: \u used with no following hex digits
+// DATA: Contents of section data:
+// DATA-NEXT:  efbeadde
+
+.warning  // required to avoid FileCheck empty input error
+.macro foo, u, name
+.section \name, "a", %progbits
+.word \u
+.endm
+foo 0xdeadbeef, data
Index: lib/Lex/Lexer.cpp
===
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -3603,17 +3603,19 @@
 
   // UCNs (C99 6.4.3, C++11 [lex.charset]p2)
   case '\\':
-if (uint32_t CodePoint = tryReadUCN(CurPtr, BufferPtr, &Result)) {
-  if (CheckUnicodeWhitespace(Result, CodePoint, CurPtr)) {
-if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
-  return true; // KeepWhitespaceMode
+if (!LangOpts.AsmPreprocessor) {
+  if (uint32_t CodePoint = tryReadUCN(CurPtr, BufferPtr, &Result)) {
+if (CheckUnicodeWhitespace(Result, CodePoint, CurPtr)) {
+  if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
+return true; // KeepWhitespaceMode
+
+  // We only saw whitespace, so just try again with this lexer.
+  // (We manually eliminate the tail call to avoid recursion.)
+  goto LexNextToken;
+}
 
-// We only saw whitespace, so just try again with this lexer.
-// (We manually eliminate the tail call to avoid recursion.)
-goto LexNextToken;
+return LexUnicode(Result, CodePoint, CurPtr);
   }
-
-  return LexUnicode(Result, CodePoint, CurPtr);
 }
 
 Kind = tok::unknown;


Index: test/Lexer/asm-preproc-no-unicode.s
===
--- /dev/null
+++ test/Lexer/asm-preproc-no-unicode.s
@@ -0,0 +1,13 @@
+// RUN: %clang --target=arm-arm-none-eabi -c -xassembler-with-cpp %s -o %t 2>&1 | FileCheck %s --check-prefix=WARNING
+// RUN: llvm-objdump -s %t | FileCheck %s --check-prefix=DATA
+
+// WARNING-NOT: warning: \u used with no following hex digits
+// DATA: Contents of section data:
+// DATA-NEXT:  efbeadde
+
+.warning  // required to avoid FileCheck empty input error
+.macro foo, u, name
+.section \name, "a", %progbits
+.word \u
+.endm
+foo 0xdeadbeef, data
Index: lib/Lex/Lexer.cpp
===
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -3603,17 +3603,19 @@
 
   // UCNs (C99 6.4.3, C++11 [lex.charset]p2)
   case '\\':
-if (uint32_t CodePoint = tryReadUCN(CurPtr, BufferPtr, &Result)) {
-  if (CheckUnicodeWhitespace(Result, CodePoint, CurPtr)) {
-if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
-  return true; // KeepWhitespaceMode
+if (!LangOpts.AsmPreprocessor) {
+  if (uint32_t CodePoint = tryReadUCN(CurPtr, BufferPtr, &Result)) {
+if (CheckUnicodeWhitespace(Result, CodePoint, CurPtr)) {
+  if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
+return true; // KeepWhitespaceMode
+
+  // We only saw whitespace, so just try again with this lexer.
+  // (We manually eliminate the tail call to avoid recursion.)
+  goto LexNextToken;
+}
 
-// We only saw whitespace, so just try again with this lexer.
-// (We manually eliminate the tail call to avoid recursion.)
-goto LexNextToken;
+return LexUnicode(Result, CodePoint, CurPtr);
   }
-
-  return LexUnicode(Result, CodePoint, CurPtr);
 }
 
 Kind = tok::unknown;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31765: Skip Unicode character expansion in assembly files

[Salman Arif via Phabricator via cfe-commits]

salari01 updated this revision to Diff 94501.
salari01 added a comment.

Updated test to check preprocessed output instead of the assembled file. Cannot 
use `-verify` with the driver, but with `-E` and `-o -`, there is no longer a 
need to have the dummy warning to avoid the FileCheck error.


https://reviews.llvm.org/D31765

Files:
  lib/Lex/Lexer.cpp
  test/Lexer/asm-preproc-no-unicode.s


Index: test/Lexer/asm-preproc-no-unicode.s
===
--- /dev/null
+++ test/Lexer/asm-preproc-no-unicode.s
@@ -0,0 +1,8 @@
+// RUN: %clang -E -xassembler-with-cpp %s -o - 2>&1 | FileCheck %s
+
+// CHECK-NOT: warning: \u used with no following hex digits
+// CHECK: .word \u
+
+.macro foo, u
+.word \u
+.endm
Index: lib/Lex/Lexer.cpp
===
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -3603,17 +3603,19 @@
 
   // UCNs (C99 6.4.3, C++11 [lex.charset]p2)
   case '\\':
-if (uint32_t CodePoint = tryReadUCN(CurPtr, BufferPtr, &Result)) {
-  if (CheckUnicodeWhitespace(Result, CodePoint, CurPtr)) {
-if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
-  return true; // KeepWhitespaceMode
+if (!LangOpts.AsmPreprocessor) {
+  if (uint32_t CodePoint = tryReadUCN(CurPtr, BufferPtr, &Result)) {
+if (CheckUnicodeWhitespace(Result, CodePoint, CurPtr)) {
+  if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
+return true; // KeepWhitespaceMode
+
+  // We only saw whitespace, so just try again with this lexer.
+  // (We manually eliminate the tail call to avoid recursion.)
+  goto LexNextToken;
+}
 
-// We only saw whitespace, so just try again with this lexer.
-// (We manually eliminate the tail call to avoid recursion.)
-goto LexNextToken;
+return LexUnicode(Result, CodePoint, CurPtr);
   }
-
-  return LexUnicode(Result, CodePoint, CurPtr);
 }
 
 Kind = tok::unknown;


Index: test/Lexer/asm-preproc-no-unicode.s
===
--- /dev/null
+++ test/Lexer/asm-preproc-no-unicode.s
@@ -0,0 +1,8 @@
+// RUN: %clang -E -xassembler-with-cpp %s -o - 2>&1 | FileCheck %s
+
+// CHECK-NOT: warning: \u used with no following hex digits
+// CHECK: .word \u
+
+.macro foo, u
+.word \u
+.endm
Index: lib/Lex/Lexer.cpp
===
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -3603,17 +3603,19 @@
 
   // UCNs (C99 6.4.3, C++11 [lex.charset]p2)
   case '\\':
-if (uint32_t CodePoint = tryReadUCN(CurPtr, BufferPtr, &Result)) {
-  if (CheckUnicodeWhitespace(Result, CodePoint, CurPtr)) {
-if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
-  return true; // KeepWhitespaceMode
+if (!LangOpts.AsmPreprocessor) {
+  if (uint32_t CodePoint = tryReadUCN(CurPtr, BufferPtr, &Result)) {
+if (CheckUnicodeWhitespace(Result, CodePoint, CurPtr)) {
+  if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
+return true; // KeepWhitespaceMode
+
+  // We only saw whitespace, so just try again with this lexer.
+  // (We manually eliminate the tail call to avoid recursion.)
+  goto LexNextToken;
+}
 
-// We only saw whitespace, so just try again with this lexer.
-// (We manually eliminate the tail call to avoid recursion.)
-goto LexNextToken;
+return LexUnicode(Result, CodePoint, CurPtr);
   }
-
-  return LexUnicode(Result, CodePoint, CurPtr);
 }
 
 Kind = tok::unknown;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31496: Make -defsym a driver option

[Salman Arif via Phabricator via cfe-commits]

salari01 abandoned this revision.
salari01 added a comment.

In https://reviews.llvm.org/D31496#721262, @rnk wrote:

> `-Wl,--defsym` is also a pretty common linker option. I'm hesitant to add 
> this to the driver interface, since it seems like they could easily be 
> confused.


I see. Was not aware of that linker option. Having looked it up now, I agree 
that the risk of confusion with the linker option outweighs the convenience of 
having this as a Driver option. I'll adandon this revision.


https://reviews.llvm.org/D31496



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