aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
In general, I think this looks good (I did find one minor nit that removes an
include, but that can be fixed as you land). There's an open question for
@jansvoboda11 and it'd be nice to hear if @joerg agrees their concerns are
fully addressed, so please wait a bit before landing to give them time to
respond with any concerns or feedback.
================
Comment at: clang/include/clang/Driver/Options.td:4300
defm pack_derived : BooleanFFlag<"pack-derived">, Group<gfortran_Group>;
-defm protect_parens : BooleanFFlag<"protect-parens">, Group<gfortran_Group>;
+//defm protect_parens : BooleanFFlag<"protect-parens">, Group<gfortran_Group>;
defm range_check : BooleanFFlag<"range-check">, Group<gfortran_Group>;
----------------
mibintc wrote:
> @jansvoboda11 This is a gfortran option, but I don't think there's a way to
> allow the same option spelling for gfortran and clang. Other clang options,
> like -short-enum, also have been pulled out of gfortran group, and the
> gfortran option test is an expected-fail test.
@jansvoboda11 -- do you have any ideas here? Or is the current approach fine
(and we can remove this commented-out line)?
================
Comment at: clang/include/clang/Sema/Sema.h:5428
bool AllowRecovery = false);
+ Expr *BuildBuiltinCallExpr(SourceLocation Loc, Builtin::ID Id,
+ MultiExprArg CallArgs);
----------------
I think it might make sense to pass in an `unsigned` rather than a
`Builtin::ID` so that we can avoid including `Builtins.h` (we do this sort of
dance to avoid including headers elsewhere as well). WDYT?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100118/new/
https://reviews.llvm.org/D100118
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
