MForster marked an inline comment as done. MForster added inline comments.
================ Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5350 + + D->addAttr(::new (S.Context) + NSErrorDomainAttr(S.Context, AL, IdentLoc->Ident)); ---------------- aaron.ballman wrote: > MForster wrote: > > aaron.ballman wrote: > > > MForster wrote: > > > > aaron.ballman wrote: > > > > > Shouldn't we also be validating that what we found is an NSString > > > > > that has the correct properties? (That doesn't seem like it should be > > > > > a change which risks breaking code based on what I understood from > > > > > Doug's description.) > > > > > Shouldn't we also be validating that what we found is an NSString > > > > > that has the correct properties? > > > > > > > > Is your suggestion to string-compare the name of the type? > > > >>Shouldn't we also be validating that what we found is an NSString that > > > >>has the correct properties? > > > > Is your suggestion to string-compare the name of the type? > > > > > > You should be able to compare the `QualType` of the resulting `VarDecl` > > > against `ASTContext::getObjCNSStringType()` (accounting for qualifiers, > > > pointers, etc). > > Turns out that this didn't work well. `ASTContext::getObjCNSStringType()` > > is only initialized if ObjC string literals are instantiated without an > > `NSString` type being defined. In our case there is an `NSString` type, > > because we need to declare a global variable of that type. > > > > I've resorted to a string comparison after all. > Well that's really unfortunate to learn, but good news: `isNSStringType()` is > in SemaDeclAttr.cpp and I hadn't noticed that before, so I think you can use > that instead, assuming that a mutable string is acceptable for the API. If > mutable strings are not acceptable, then I think we should add a new > parameter to `isNSStringType()` to handle it. Nice find. Should have found this myself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84005/new/ https://reviews.llvm.org/D84005 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
