aaron.ballman added inline comments.
================
Comment at: clang/examples/Attribute/Attribute.cpp:26
ExampleAttrInfo() {
- // Can take an optional string argument (the check that the argument
- // actually is a string happens in handleDeclAttribute).
- OptArgs = 1;
+ // Can take variadic arguments.
+ OptArgs = 15;
----------------
The comment is a bit misleading, how about: `Can take up to 15 optional
arguments, to emulate accepting a variadic number of arguments.`?
================
Comment at: clang/examples/Attribute/Attribute.cpp:57
}
- // Check if we have an optional string argument.
- StringRef Str = "";
+ // Make sure there are at most three arguments exists
+ if (Attr.getNumArgs() > 3) {
----------------
This comment describes what the code is doing but not why. How about: `The
example attribute supports up to 15 optional arguments, but this demonstrates
how to test for a specific number of arguments.` or something along those lines?
================
Comment at: clang/examples/Attribute/Attribute.cpp:65
+ }
+ // If has arguments, the first argument should be a string literal
+ Expr *Arg0 = nullptr;
----------------
If has arguments -> If there are arguments
(And add a full stop to the end of the comment.)
================
Comment at: clang/examples/Attribute/Attribute.cpp:81
+ D->addAttr(AnnotateAttr::Create(S.Context, "example", &Arg0,
+ Attr.getNumArgs(), Attr.getRange()));
return AttributeApplied;
----------------
This looks dangerous to me -- if there are two or three arguments supplied,
then this code will result in a buffer overrun.
================
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
----------------
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.
================
Comment at: clang/test/Frontend/plugin-attribute.cpp:12
-
-// ATTRIBUTE: [[STR1_VAR:@.+]] = private unnamed_addr constant [10 x i8]
c"example()\00"
-// ATTRIBUTE: [[STR2_VAR:@.+]] = private unnamed_addr constant [20 x i8]
c"example(somestring)\00"
----------------
A downside to this is that we lose the verification that the plugin actually
attaches the attribute to the AST node. Another solution that doesn't involve
generating LLVM bitcode from a Frontend test would be to use `-ast-dump` and
check the AST nodes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92006/new/
https://reviews.llvm.org/D92006
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
