xazax.hun added a comment. In D63954#1565109 <https://reviews.llvm.org/D63954#1565109>, @gribozavr wrote:
> > I agree. In a follow-up patch we will add the attributes to STL types > > during parsing. Do you think it is OK to always add the attributes or > > should we only do it if a flag, e.g. -wlifetime is added to the compiler > > invocation? > > Warning flags should not change the AST -- compiler engineers don't expect > that. So if Clang is going to perform inference, it should either always do > it, or it should be guarded by an `-f` flag, not a `-W` flag. Thanks, this make sense. My only concern would be that a -W flag without the "inference" for STL types would be useless. Is it ok to make the driver add a -f flag automatically if a warning is turned on or would you find that confusing as well? > > >> On the other hand I still think it is useful to give the users the option to >> maintain a header with forward declarations to add the annotations to other >> (non-standard) 3rd party types. These headers might be fragile to 3rd party >> changes but could still be a better option than maintaining patches on top >> of 3rd party libraries. Having API notes upstream would be a superior >> solution here and I might look into upstreaming it at some point, but I >> think this is something that could be addressed later on. What do you think? > > I think it is acceptable for 3rd party libraries, but there's already a > solution for 3rd party libraries -- API notes in Swift's fork of Clang. It > has been successfully used by Apple for 5 years for this exact purpose > (adding attributes to existing libraries without changing headers), and only > needs to be upstreamed to Clang. Supporting the forward declarations way is only a few lines of code now, supporting API Notes is a much larger effort. I would also prefer the latter but I think having this work not blocked on upstreaming API notes is essential to get this upstreamed. Or would you prefer not supporting the 3rd party library use-case until API Notes are upstreamed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
