[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Frontend/HeaderIncludeGen.cpp:55
+  // Simplify Filename that starts with "./"
+  if (Filename.startswith("./"));
+    Filename=Filename.substr(2);
----------------
skan wrote:
> lebedev.ri wrote:
> > skan wrote:
> > > craig.topper wrote:
> > > > skan wrote:
> > > > > lebedev.ri wrote:
> > > > > > xiangzhangllvm wrote:
> > > > > > > Need remove ";" ? 
> > > > > > This was fixed but no test was added?
> > > > > > Was some existing test failing previously? Which one?
> > > > > The test is in the file 'clang_H_opt.c'  which is included in this patch.
> > > > The extra semicolon would have caused the body of the 'if' to execute unconditionally. Did any existing test case fail for that in your local testing? Or did you not test with that mistake?
> > > i fixed the mistake in the updated patch.  I ran the test in 'clang_H_opt.c' alone for this patch. The extra semicolon caused the body of `if` to exeute always, which didn't cause the test to fail. 
> > > The extra semicolon caused the body of if to exeute always, which didn't cause the test to fail.
> > 
> > That is the question.
> > Is there test coverage with that error?
> No. I just run alll the tests for clang, and all of them can pass even if the extra semicolon exits. If we want to add  a test to cover that error, we have to add a headerfile outside the 'test/Driver' directory. Do we need to add the test to cover that error?
Then yes please, do add test coverage for that bug :)


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

https://reviews.llvm.org/D62115