rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/include/clang/Parse/Parser.h:804-807
+ bool TryAnnotateTypeOrScopeToken(bool ClassTemplateDeductionContext = true);
+ bool TryAnnotateTypeOrScopeTokenAfterScopeSpec(
+ CXXScopeSpec &SS, bool IsNewScope,
+ bool ClassTemplateDeductionContext = true);
----------------
Looks like the flag you're adding here is never actually used any more. Please
remove it again.
================
Comment at: clang/lib/AST/StmtPrinter.cpp:2296
+ S.flush();
+ if (Buf.find("typename ") != 0)
+ OS << "typename ";
----------------
rsmith wrote:
> I can see why this is the most straightforward way to implement this, but ...
> yuck. Please add a FIXME :)
I think you've actually fixed this now: we always build a `typename` type, so
never need to print an extra `typename` here.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:8291-8292
+
+ // Maintain an efficient lookup of params we have seen so far.
+ llvm::SmallSet<const IdentifierInfo*, 16> ParamsSoFar;
+
----------------
This variable is now unused; please remove it.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:8309
+ CheckShadow(BodyScope, Param);
+ PushOnScopeChains(Param, BodyScope);
+ }
----------------
If we only do this now, do we properly handle `requires (int x, decltype(x)
y)`? (Or is that handled in a separate function prototype scope which we've
already left?)
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:10259-10261
+ else
+ Diag(CondRange.getBegin(), diag::err_typename_not_found_enable_if)
+ << CondRange;
----------------
Minor cleanup: I don't think we can ever reasonably get here for
`enable_if<...>::type` with no `Ctx`. I mean, it's possible, but only from
lexically within the definition of `enable_if` itself (or possibly a class
derived from it?), and that's really not worth checking for. We could just skip
calling `isEnableIf` and this whole `if` block if we don't have a `Ctx`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D50360/new/
https://reviews.llvm.org/D50360
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
