jdoerfert added inline comments.
================
Comment at: clang/lib/AST/OpenMPClause.cpp:1723
+ llvm::APInt CondVal =
+ Selector.ScoreOrCondition->EvaluateKnownConstInt(ASTCtx);
+ if (CondVal.isNullValue())
----------------
aaron.ballman wrote:
> jdoerfert wrote:
> > aaron.ballman wrote:
> > > I didn't see anything that validated that this is known to be a constant
> > > expression. Did I miss something?
> > >
> > > Also, is there anything checking that the expression is not value
> > > dependent?
> > > I didn't see anything that validated that this is known to be a constant
> > > expression. Did I miss something?
> >
> > I'll look into that and make sure we check in the parser.
> >
> > > Also, is there anything checking that the expression is not value
> > > dependent?
> >
> > That should be fine. If it is attached to a template it is instantiated
> > with the template. Or am I missing something?
> > That should be fine. If it is attached to a template it is instantiated
> > with the template. Or am I missing something?
>
> `EvaluateKnownConstInt()` asserts that the expression is not value dependent,
> so I just wanted to be certain that assertion wouldn't trigger. Sounds like
> it won't because this should only be called on an instantiation?
I added detection and tests for constant, value-dependent, and non-constant
expressions in score and user condition. Assuming I didn't miss anything it
should not assert here.
================
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:
> 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.
================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:110
.Case("ParamIdx", "ParamIdx::deserialize(Record.readInt())")
+ .EndsWith("*", "Record.readUserType<" +
+ std::string(type.data(), 0, type.size() - 1) + ">()")
----------------
aaron.ballman wrote:
> jdoerfert wrote:
> > aaron.ballman wrote:
> > > The idea here being that if someone adds a `GenericPointerArg`, they'll
> > > have to add a specialization of `readUserType<>` for that named pointer
> > > type or else get template instantiation errors because there's no
> > > definition for the primary template?
> > Correct.
> I like it. :-)
It's gone now ;)
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
