[PATCH] D92006: Refactoring the attrubute plugin example to fit the new API

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/examples/Attribute/Attribute.cpp:65
+          DiagnosticsEngine::Error,
+          "'example' attribute only allowed at most three arguments");
+      S.Diag(Attr.getLoc(), ID);
----------------
only allowed -> accepts


================
Comment at: clang/examples/Attribute/Attribute.cpp:77
+            DiagnosticsEngine::Error,
+            "'example's first argument should be a string literal");
+        S.Diag(Attr.getLoc(), ID);
----------------
How about: `first argument to the 'example' attribute must be a string literal`


================
Comment at: clang/test/Frontend/plugin-attribute.cpp:1
-// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm -S %s -o - 2>&1 | FileCheck %s --check-prefix=ATTRIBUTE
-// RUN: not %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm -DBAD_ATTRIBUTE -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADATTRIBUTE
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -DGOOD_ATTR -fsyntax-only -Xclang -verify=goodattr %s
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -DBAD_ATTR -fsyntax-only -Xclang -verify=badattr %s
----------------
psionic12 wrote:
> aaron.ballman wrote:
> > I don't think we need a GOOD_ATTR and BAD_ATTR run line any longer. Instead, you can use a single RUN line with `-verify` (no `=`) and use `expected-error` and `expected-warning` on the lines that need the diagnostics. This means you can also get rid of the `@ line number` constructs and just write the expected comments on the source lines with diagnostics.
> Seems the `GOOD_ATTR` and `BAD_ATTR` need to be exist since we use `-ast-dump` now, we need a clean build without error diagnostics to generate a nice AST.
That suggests this should be two test files with one RUN line apiece.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92006