aaron.ballman accepted this revision.
aaron.ballman added a comment.
LGTM aside from some small nits.
================
Comment at: clang/include/clang/Basic/Attr.td:186-192
+// an OMPTraitInfo object. The structure of an OMPTraitInfo object is a
+// tree as defined below:
+//
+// OMPTraitInfo := {list<OMPTraitSet>}
+// OMPTraitSet := {Kind, list<OMPTraitSelector>}
+// OMPTraitSelector := {Kind, Expr, list<OMPTraitProperty>}
+// OMPTraitProperty := {Kind}
----------------
Rather than describing the internal data structure form, I am hoping for
comments that show the format of the user-facing pragma arguments. e.g., what
information is in what position. Basically, something so I can mentally map
from "OMPTraitsInfoArgument" to "oh, that's right, the arguments are specified
in this order with these types" (roughly -- a general idea of the argument is
all I'm looking for).
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:1379
// Parse inner context selectors.
- SmallVector<Sema::OMPCtxSelectorData, 4> Data;
- if (!parseOpenMPContextSelectors(Loc, Data)) {
- // Parse ')'.
- (void)T.consumeClose();
- // Need to check for extra tokens.
- if (Tok.isNot(tok::annot_pragma_openmp_end)) {
- Diag(Tok, diag::warn_omp_extra_tokens_at_eol)
- << getOpenMPDirectiveName(OMPD_declare_variant);
- }
- }
+ OMPTraitInfo *TI = new OMPTraitInfo();
+ parseOMPContextSelectors(Loc, *TI);
----------------
jdoerfert wrote:
> jdoerfert wrote:
> > aaron.ballman wrote:
> > > What is responsible for freeing this memory?
> > Will check and fix if needed.
> I made sure all OMPTraitInfo are freed, I think. If an
> `OMPDeclareVariantAttr` is build its taking owenership and calls delete in
> the destructor.
That is reasonable enough -- any chance we can easily turn it into a
`std::unique_ptr<>` and then get rid of the destructor entirely? I don't
insist, but it would be cleaner, to my thinking.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71830/new/
https://reviews.llvm.org/D71830
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
