hokein added inline comments.
================
Comment at: clang/include/clang/Basic/LangOptions.def:153
COMPATIBLE_LANGOPT(RecoveryASTType, 1, 0, "Preserve the type in recovery
expressions")
+COMPATIBLE_LANGOPT(CDependence, 1, 0, "Build dependent AST nodes in C for
better error recovery")
----------------
sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > Why is this not just the RecoveryAST flag? Is there some use of
> > > dependence outside the error-handling path?
> > >
> > > If the language is C, recovery-AST only works if this flag is on too. So
> > > they might as well always have the same value.
> > > If the language is C++, this flag is meaningless, right?
> > This flag is mainly for turning off early typo-corrections for C.
> >
> > Thinking more about this, you're right, looks like using the RecoveryAST is
> > possible:
> > - these early typo-corrections applies non-C++ code, and have to be there
> > until we fully implemented recovery-expr-on-c;
> > - the current default mode of RecoveryAST: on for C++, and off for non-C++;
> > - for recovery-expr-on-c development, we need to turn on the RecoveryAST
> > and turn off these early typo-corrections;
> >
> > so `if (!S.getLangOpts().CPlusPlus && !S.getLangOpts().RecoveryAST) {`
> > works.
> >
> >
> If that's going to be a common condition, I'd suggest a helper function like
>
> ```
> /// (explanation referencing early typo correction here)
> isDependenceAllowed(const LangOpts &LO) { return CPlusPlus || RecoveryAST; }
> ```
> and then guard early typo correction with `if (!isDependenceAllowed())`
sounds a good idea.
================
Comment at: clang/include/clang/Sema/Sema.h:5169
+ /// In C++, we need to perform unqualified lookups when creating a dependent
+ /// binary operator, thus this should be only used when the unqalified lookup
+ /// found nothing.
----------------
sammccall wrote:
> nit: unqualified
> nit: remove "results"
this was in the first snapshot, we don't need this change now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84226/new/
https://reviews.llvm.org/D84226
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
