Anastasia added a comment. In D90928#2408249 <https://reviews.llvm.org/D90928#2408249>, @erik2020 wrote:
> In D90928#2405796 <https://reviews.llvm.org/D90928#2405796>, @Anastasia wrote: > >> Do you think we could improve testing? I presume there is something that >> triggers a failure without your change... > > I'm not really sure how to test this code. Best I can tell, there's no way > for the `clang` executable to call these functions with invalid strings. I > only ran into the seg faults when I was programmatically setting options > using the clang API. Oh, I see. We could also create a g-test for this but it's probably not worth enough. Perhaps if you just update a comment that the API behavior becomes clear it should be good enough. ================ Comment at: clang/include/clang/Basic/OpenCLOptions.h:47 bool isSupported(llvm::StringRef Ext, const LangOptions &LO) const { + auto E = OptMap.find(Ext); + if (E == OptMap.end()) { ---------------- Btw how about we use `isKnown` instead because it does exactly that? Also, I think we should update the comment to explain this change in the API behavior and add a comment for `isKnown`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90928/new/ https://reviews.llvm.org/D90928 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
