<div dir="ltr">The TODO in checkNameAndExistence seems relevant here. Implementing proc.type.already.exists sounds good, but I assume it would be a warning rather than a mandatory error? If so I think we should still improve the handling of overwriting symbols during annotation processing with something like the proposed fix for JDK-8193037.<br><div><br></div><div>I filed a bug about implementing proc.type.already.exists: <a href="https://bugs.openjdk.java.net/browse/JDK-8193216">https://bugs.openjdk.java.net/browse/JDK-8193216</a></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 6, 2017 at 1:11 AM, Liam Miller-Cushon <span dir="ltr"><<a href="mailto:cushon@google.com" target="_blank">cushon@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>I think the current implementation of Filer doesn't check if generated classes override arbitrary classes on the classpath, it only checks if they clash with previously generated classes, or the sources and classes that were explicit inputs to the compilation: <a href="http://hg.openjdk.java.net/jdk/jdk/file/a9405d9ca8a8/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacFiler.java#l719" target="_blank">http://hg.openjdk.java.net/<wbr>jdk/jdk/file/a9405d9ca8a8/src/<wbr>jdk.compiler/share/classes/<wbr>com/sun/tools/javac/<wbr>processing/JavacFiler.java#<wbr>l719</a></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Dec 5, 2017 at 11:48 PM, Jonathan Gibbons <span dir="ltr"><<a href="mailto:jonathan.gibbons@oracle.com" target="_blank">jonathan.gibbons@oracle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div text="#000000" bgcolor="#FFFFFF">
    <p>We'll need Joe "Mr Annotation Processing" Darcy to chime in here,
      but the Filer is supposed to detect clashes, and prevent
      overwriting/overriding symbols. That's definitely supposed to
      happen for normal classes/interfaces; I could believe that
      package-info has been overlooked, but I would expect it to follow
      the same guidelines.</p>
    <p>-- Jon<br>
    </p><div><div class="m_-2339433768679239052h5">
    <br>
    <div class="m_-2339433768679239052m_8114950477092954641moz-cite-prefix">On 12/5/17 6:24 PM, Liam Miller-Cushon
      wrote:<br>
    </div>
    <blockquote type="cite">
      <div dir="ltr">Is overriding package-infos different or worse than
        overriding other symbols? I've seen a number of examples where
        the same source file was compiled multiple times and the earlier
        results ended up on the class path of the later compilations, so
        a processor-generated class was both on the class path and
        generated during the compilation. Making that an error would be
        a somewhat invasive breaking change. I agree that it should be
        discouraged, but I'm not sure it's worth making an error?
        (Unless there's something special about package-infos that I'm
        missing.)</div>
      <div class="gmail_extra"><br>
        <div class="gmail_quote">On Tue, Dec 5, 2017 at 6:09 PM,
          Jonathan Gibbons <span dir="ltr"><<a href="mailto:jonathan.gibbons@oracle.com" target="_blank">jonathan.gibbons@oracle.com</a>></span>
          wrote:<br>
          <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
            <div bgcolor="#FFFFFF" text="#000000"> Generally, annotation
              processing is supposed to only create information where
              there was no information previously,  so if a package had
              a package-info with annotations, it seems like it should
              be an error to override it with another package-info, with
              or without annotations.<br>
              <br>
              -- Jon
              <div>
                <div class="m_-2339433768679239052m_8114950477092954641h5"><br>
                  <br>
                  <div class="m_-2339433768679239052m_8114950477092954641m_6854548390215522736moz-cite-prefix">On
                    12/05/2017 05:39 PM, Liam Miller-Cushon wrote:<br>
                  </div>
                  <blockquote type="cite">
                    <div dir="ltr">Thanks! If an annotated package-info
                      is loaded from the class path, and then a
                      processor generates a package-info that contains
                      no annotations, the reset is necessary. (The reset
                      isn't necessary if the new package-info is
                      annotated, since the old annotations get
                      overwritten even if they weren't reset between
                      rounds.)</div>
                    <div class="gmail_extra"><br>
                      <div class="gmail_quote">On Tue, Dec 5, 2017 at
                        4:46 PM, Jonathan Gibbons <span dir="ltr"><<a href="mailto:jonathan.gibbons@oracle.com" target="_blank">jonathan.gibbons@oracle.com</a>></span>
                        wrote:<br>
                        <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Liam,<br>
                          <br>
                          What about the case where an annotation
                          processor generates the package-info.java
                          file? Is that a case where it is important to
                          reinit the packge symbol correctly, so that
                          the newly generated code is read?<br>
                          <br>
                          -- Jon
                          <div class="m_-2339433768679239052m_8114950477092954641m_6854548390215522736HOEnZb">
                            <div class="m_-2339433768679239052m_8114950477092954641m_6854548390215522736h5"><br>
                              <br>
                              On 12/05/2017 03:39 PM, Liam Miller-Cushon
                              wrote:<br>
                              <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I have a
                                question about the logic in
                                JavacProcessingEnvironment's treeCleaner
                                that resets package-info state between
                                annotation processing rounds [1].<br>
                                <br>
                                JDK-8193037 describes an issue where
                                package-infos loaded from the classpath
                                are reset by treeCleaner. Those symbols
                                doesn't get reinitialized correctly, and
                                package annotations are not visible
                                during subsequent annotation processing
                                rounds.<br>
                                <br>
                                I wondered if the logic was only meant
                                to apply to package-infos being compiled
                                from source in the current compilation
                                (similar to how module-infos are handle
                                by treeCleaner), but I'm having trouble
                                understanding when that logic is
                                necessary. Commenting out
                                `node.packge.package_info.rese<wbr>t();`
                                and `node.packge.reset();` in
                                treeCleaner doesn't break any jtreg
                                tests. Does anyone have examples where
                                that code is needed? I'd like to add a
                                regression test to ensure the fix for
                                JDK-8193037 doesn't interfere with the
                                original purpose of that code.<br>
                                <br>
                                Thanks,<br>
                                <br>
                                [1] <a href="http://hg.openjdk.java.net/jdk/jdk/file/a358ebcfacfb/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l1518" rel="noreferrer" target="_blank">http://hg.openjdk.java.net/jdk<wbr>/jdk/file/a358ebcfacfb/src/jdk<wbr>.compiler/share/classes/com/su<wbr>n/tools/javac/processing/Javac<wbr>ProcessingEnvironment.java#l15<wbr>18</a><br>
                              </blockquote>
                              <br>
                            </div>
                          </div>
                        </blockquote>
                      </div>
                      <br>
                    </div>
                  </blockquote>
                  <br>
                </div>
              </div>
            </div>
          </blockquote>
        </div>
        <br>
      </div>
    </blockquote>
    <br>
  </div></div></div>

</blockquote></div><br></div>
</div></div></blockquote></div><br></div>