mikael marked 3 inline comments as done. mikael added inline comments.
================ Comment at: lib/AST/Type.cpp:2950 + FunctionTypeBits.HasExtQuals = 0; + } } ---------------- rjmccall wrote: > The indentation here is messed up. > > You seem to be mixing up "fast qualifiers" with "CVR qualifiers" in a few > places. That's currently correct, in the sense that the fast qualifiers are > currently defined to be the CVR qualifiers, but the point of the distinction > is that we might want to change that (and we have in fact explored that in > the past). I think you should make `FunctionTypeBits` only claim to store > the fast qualifiers, which mostly just means updating a few field / accessor > names and changing things like the `getCVRQualifiers()` call right above this > to be `getFastQualifiers()`. > > If there are places where it's convenient to assume that the CVR qualifiers > are exactly the fast qualifiers, it's okay to static_assert that, but you > should make sure to static_assert it in each place you assume it. I'll change it, but I do have a question related to this: Can we make any assumptions with the "fast qualifiers"? I'd like it if we can assume that they fit in 3 bits. Otherwise I think I also need an assertion here. ================ Comment at: lib/CodeGen/CGDebugInfo.cpp:2593 + getOrCreateInstanceMethodType( + CXXMethodDecl::getThisType(FPT, Ty->getMostRecentCXXRecordDecl()), + FPT, U), ---------------- rjmccall wrote: > Why is this `getMostRecentCXXRecordDecl` instead of `getClass`? Your initial comment suggested to send in a record rather than a type. But I see now that it may make more sense to use the type directly instead. Will update it. ================ Comment at: lib/CodeGen/CGDeclCXX.cpp:32 + (D.hasLocalStorage() && CGF.getContext().getLangOpts().OpenCLCPlusPlus)) && + "VarDecl must have global or local (in the case of OpenCL) storage!"); assert(!D.getType()->isReferenceType() && ---------------- rjmccall wrote: > What causes OpenCL C++ declarations to go down this path? It goes there when we defined an object in __local address space. From the spec "The constructors of a objects in local memory should only be initialized once". This is the same case as with C++ 11 static local variables. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54862/new/ https://reviews.llvm.org/D54862 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
