gribozavr added inline comments.

================
Comment at: clang/lib/Sema/SemaInit.cpp:6581
+    if (!Callee->getIdentifier()) {
+      auto OO = Callee->getOverloadedOperator();
+      return OO == OverloadedOperatorKind::OO_Subscript ||
----------------
xazax.hun wrote:
> gribozavr wrote:
> > mgehre wrote:
> > > xazax.hun wrote:
> > > > If we want to relax the warnings to give more results we could extend 
> > > > the checking of these overloaded operators for annotated types. But 
> > > > this would imply that the user need to have the expected semantics for 
> > > > those types and can only suppress false positives by removing some 
> > > > gsl:::owner/poinnter annotations.
> > > I see those options:
> > > - Either gsl::Owner implies some specific form of those operators (and if 
> > > that does not hold for a class, then one should not annotate it with 
> > > gsl::Owner)
> > > - or gsl::Owner only implies some specific behavior for the "gsl::Pointer 
> > > constructed from gsl::Owner" case and everything else requires additional 
> > > annotation
> > > I expect that we will experiment a bit in the future to see what works 
> > > well for real-world code.
> > I understand the difficulty, but I don't think it is appropriate to 
> > experiment by ourselves -- these attributes are defined in a spec, and if 
> > something is not clear, the spec should be clarified.
> This is exactly what is going to happen but I think it would be unfortunate 
> to stall the progress until the new version of the spec materializes. The 
> idea is to keep the implementations and the specs in sync, but as Herb has 
> other projects too, it takes some time to channel the experience back into 
> the spec. As the current version of the warnings found true positives in real 
> world projects and we have yet to see any false positives I would prefer to 
> move forward to maximize utility.  
> 
> 
I don't understand how different implementations can ever converge in that case.

If this language extension is not sufficiently designed yet, maybe it is not 
ready for inclusion in Clang?




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65127/new/

https://reviews.llvm.org/D65127



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to