aaron.ballman added a comment.
In http://reviews.llvm.org/D20352#482590, @dberris wrote:
> It looks like I was using `hasArg` instead of `hasFlag`. The dangerous part
> here is that OptSpecifier has an unsigned non-explicit argument, and I
> suspect `bool` is being promoted to `unsigned` silent
dberris added a comment.
It looks like I was using `hasArg` instead of `hasFlag`. The dangerous part
here is that OptSpecifier has an unsigned non-explicit argument, and I suspect
`bool` is being promoted to `unsigned` silently with clang/gcc.
http://reviews.llvm.org/D20352
dberris updated this revision to Diff 63775.
dberris added a comment.
- Use hasFlag instead of hasArg
http://reviews.llvm.org/D20352
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen
dberris updated this revision to Diff 63771.
dberris added a comment.
Rebase
http://reviews.llvm.org/D20352
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/CodeGenFunction.cpp
li
aaron.ballman added a comment.
I applied your patch locally and got a few compile errors with MSVC 2015:
'bool
llvm::opt::ArgList::hasArg(llvm::opt::OptSpecifier,llvm::opt::OptSpecifier,llvm::opt::OptSpecifier)
const': cannot convert argument 3 from 'bool' to 'llvm::opt::OptSpecifier'
E:\ll
dberris added a comment.
Thanks -- I don't have commit powers yet, do either of you mind landing this
for me?
Cheers
http://reviews.llvm.org/D20352
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
In http://reviews.llvm.org/D20352#477203, @rnk wrote:
> In http://reviews.llvm.org/D20352#477084, @aaron.ballman wrote:
>
> > No, I'm not. I am worried about how this conflicts wi
echristo added a comment.
In http://reviews.llvm.org/D20352#477203, @rnk wrote:
> In http://reviews.llvm.org/D20352#477084, @aaron.ballman wrote:
>
> > No, I'm not. I am worried about how this conflicts with another in-flight
> > patch for supporting MS hotpatchable functions since it seems thes
rnk added a comment.
In http://reviews.llvm.org/D20352#477084, @aaron.ballman wrote:
> No, I'm not. I am worried about how this conflicts with another in-flight
> patch for supporting MS hotpatchable functions since it seems these two
> attributes do roughly the same thing. I'd like to understa
aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.
In http://reviews.llvm.org/D20352#477038, @rnk wrote:
> Looks good from a driver perspective. Aaron, you're happy with the attribute
> spelling stuff, right?
No, I'm
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
Looks good from a driver perspective. Aaron, you're happy with the attribute
spelling stuff, right?
It looks like you can actually land this before landing the two dependent
changes you mention and
dberris updated this revision to Diff 63041.
dberris added a comment.
- Check D is valid before using it
http://reviews.llvm.org/D20352
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/Code
dberris updated this revision to Diff 63039.
dberris added a comment.
Undo previous change -- updated the wrong patch. :(
http://reviews.llvm.org/D20352
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptio
dberris updated this revision to Diff 63038.
dberris added a comment.
- Check first whether `D` is actually not nullptr
http://reviews.llvm.org/D20352
Files:
lib/CodeGen/CodeGenFunction.cpp
lib/Driver/Tools.cpp
Index: lib/Driver/Tools.cpp
===
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:436
@@ +435,3 @@
+ CXX11<"clang", "xray_never_instrument">];
+ let Subjects = SubjectList<[CXXMethod, ObjCMethod, Function], WarnDiag,
+ "ExpectedFunctionOr
dberris updated this revision to Diff 62824.
dberris marked an inline comment as done.
dberris added a comment.
- Formatting changes
http://reviews.llvm.org/D20352
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Driver/Options.td
include/clang/Frontend/C
dberris marked 2 inline comments as done.
Comment at: include/clang/Basic/Attr.td:436
@@ +435,3 @@
+ CXX11<"clang", "xray_never_instrument">];
+ let Subjects = SubjectList<[CXXMethod, ObjCMethod, Function], WarnDiag,
+ "ExpectedFunct
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:436
@@ +435,3 @@
+ CXX11<"clang", "xray_never_instrument">];
+ let Subjects = SubjectList<[CXXMethod, ObjCMethod, Function], WarnDiag,
+ "ExpectedFunctionOr
dberris updated this revision to Diff 62720.
dberris added a comment.
Rebase harder.
http://reviews.llvm.org/D20352
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/CodeGenFunction.
dberris added inline comments.
Comment at: include/clang/Basic/Attr.td:436
@@ +435,3 @@
+ CXX11<"clang", "xray_never_instrument">];
+ let Subjects = SubjectList<[CXXMethod, ObjCMethod, Function], WarnDiag,
+ "ExpectedFunctionOrMethod
dberris updated this revision to Diff 62056.
dberris marked 2 inline comments as done.
dberris added a comment.
- Undo formatting changes
- Re-apply changes post-review
- Address more comments
http://reviews.llvm.org/D20352
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
aaron.ballman added a comment.
It looks like you ran clang-format over the entire file in some instances,
causing a lot of unrelated changes to creep in. Can you back those changes out
and only run clang-format over just your patch instead
(http://clang.llvm.org/docs/ClangFormat.html#script-for
dberris marked an inline comment as done.
Comment at: include/clang/Basic/Attr.td:432
@@ +431,3 @@
+ CXX11<"clang", "xray_always_instrument">];
+ let Subjects = SubjectList<[CXXMethod, Function], WarnDiag,
+ "ExpectedFunctionOrMethod
dberris updated this revision to Diff 61932.
dberris marked 2 inline comments as done.
dberris added a comment.
- Address review comments; see details.
http://reviews.llvm.org/D20352
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Driver/Options.td
inclu
dberris updated this revision to Diff 61567.
dberris added a comment.
- Merge branch 'master' of http://llvm.org/git/clang into xray
- Merge branch 'master' of http://llvm.org/git/clang into xray
- Add runtime support for XRay
http://reviews.llvm.org/D20352
Files:
include/clang/Basic/Attr.td
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:431
@@ +430,3 @@
+ CXX11<"clang", "xray_always_instrument">];
+ let Subjects = SubjectList<[CXXMethod, Function], WarnDiag,
+ "ExpectedFunctionOrMethod">;
-
echristo added a reviewer: aaron.ballman.
echristo added a comment.
Couple of inline comments. Also adding Aaron since he normally reviews the
attributes :)
-eric
Comment at: include/clang/Driver/Options.td:753
@@ +752,3 @@
+ HelpText<"Generate XRay instrumentation sleds on f
27 matches
Mail list logo
