[PATCH] D106201: [clangd] Add tests covering existing header-guard behavior. NFC

Mon Jul 19 10:05:34 PDT 2021

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks!

================
Comment at: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp:725
+  TU.Code = R"cpp(
+    #pragma once
+    ;
----------------
sammccall wrote:
> kadircet wrote:
> > ```
> > #include "self.h"
> > #pragma once
> > ```
> > 
> > might also be an interesting case (with preamble/main file split variations). I think all of these should raise a warning for sure, I don't think we should mark these as pragma guarded. (interestingly clangd actually somewhat works on this case today, but it feels like an accident and this code won't actually compile, so I don't think preserving clangd's current behviour would be beneficial to anyone).
> Done, but only with a couple of splits, as I don't think we can cover all these edge cases exhaustively and the main behavior (diagnostic) is kinda obvious.
> 
> > I don't think we should mark these as pragma guarded
> 
> I can't see any principled reason (or way) to make them not pragma guarded.
> `#pragma once` at the end of a file is a perfectly valid header guard and not observably different from having it at the top unless the file transitively includes itself.
sorry i was thinking about the header guards appearing after the self-include not the pragma case, when talking about not marking them as pragma guarded. so it all makes sense, thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106201