Skip to content

Commit 18ae7a7

Browse files
The "interesting" part of the -output-dir, etc. PR.
Code changes, dedicated regression tests for fixes/enhancements, and other closely related changes. See the PR description for details.
1 parent 2fb795f commit 18ae7a7

23 files changed

+549
-255
lines changed

clang-tools-extra/clangd/tool/ClangdMain.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -486,8 +486,16 @@ int main(int argc, char *argv[]) {
486486

487487
// See clang/docs/checkedc/3C/clang-tidy.md#_3c-name-prefix
488488
// NOLINTNEXTLINE(readability-identifier-naming)
489-
_3CInterface _3CInterface(CcOptions, OptionsParser.getSourcePathList(),
490-
&(OptionsParser.getCompilations()));
489+
std::unique_ptr<_3CInterface> _3CInterfacePtr(
490+
_3CInterface::create(CcOptions, OptionsParser.getSourcePathList(),
491+
&(OptionsParser.getCompilations())));
492+
if (!_3CInterfacePtr) {
493+
// _3CInterface::create has already printed an error message. Just exit.
494+
return 1;
495+
}
496+
// See clang/docs/checkedc/3C/clang-tidy.md#_3c-name-prefix
497+
// NOLINTNEXTLINE(readability-identifier-naming)
498+
_3CInterface &_3CInterface = *_3CInterfacePtr;
491499
#else
492500
llvm::cl::ParseCommandLineOptions(
493501
argc, argv,

clang/include/clang/3C/3C.h

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ struct _3COptions {
3232
bool Verbose;
3333

3434
std::string OutputPostfix;
35+
std::string OutputDir;
3536

3637
std::string ConstraintOutputJson;
3738

@@ -50,6 +51,7 @@ struct _3COptions {
5051
bool EnablePropThruIType;
5152

5253
std::string BaseDir;
54+
bool AllowSourcesOutsideBaseDir;
5355

5456
bool EnableAllTypes;
5557

@@ -86,9 +88,18 @@ class _3CInterface {
8688
// Mutex for this interface.
8789
std::mutex InterfaceMutex;
8890

89-
_3CInterface(const struct _3COptions &CCopt,
90-
const std::vector<std::string> &SourceFileList,
91-
clang::tooling::CompilationDatabase *CompDB);
91+
// If the parameters are invalid, this function prints an error message to
92+
// stderr and returns null.
93+
//
94+
// There's no way for a constructor to report failure (we do not use
95+
// exceptions), so use a factory method instead. Ideally we'd use an
96+
// "optional" datatype that doesn't force heap allocation, but the only such
97+
// datatype that is accepted in our codebase
98+
// (https://llvm.org/docs/ProgrammersManual.html#fallible-constructors) seems
99+
// too unwieldy to use right now.
100+
static _3CInterface *create(const struct _3COptions &CCopt,
101+
const std::vector<std::string> &SourceFileList,
102+
clang::tooling::CompilationDatabase *CompDB);
92103

93104
// Constraint Building.
94105

@@ -121,6 +132,10 @@ class _3CInterface {
121132
bool writeConvertedFileToDisk(const std::string &FilePath);
122133

123134
private:
135+
_3CInterface(const struct _3COptions &CCopt,
136+
const std::vector<std::string> &SourceFileList,
137+
clang::tooling::CompilationDatabase *CompDB, bool &Failed);
138+
124139
// Are constraints already built?
125140
bool ConstraintsBuilt;
126141
void invalidateAllConstraintsWithReason(Constraint *ConstraintToRemove);

clang/include/clang/3C/3CGlobalOptions.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
extern bool Verbose;
1919
extern bool DumpIntermediate;
20+
extern std::string OutputPostfix;
21+
extern std::string OutputDir;
2022
extern bool HandleVARARGS;
2123
extern bool SeperateMultipleFuncDecls;
2224
extern bool EnablePropThruIType;

clang/include/clang/3C/RewriteUtils.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,15 +238,13 @@ class ArrayBoundsRewriter {
238238

239239
class RewriteConsumer : public ASTConsumer {
240240
public:
241-
explicit RewriteConsumer(ProgramInfo &I, std::string &OPostfix)
242-
: Info(I), OutputPostfix(OPostfix) {}
241+
explicit RewriteConsumer(ProgramInfo &I) : Info(I) {}
243242

244243
virtual void HandleTranslationUnit(ASTContext &Context);
245244

246245
private:
247246
ProgramInfo &Info;
248247
static std::map<std::string, std::string> ModifiedFuncSignatures;
249-
std::string &OutputPostfix;
250248

251249
// A single header file can be included in multiple translations units. This
252250
// set ensures that the diagnostics for a header file are not emitted each

clang/include/clang/3C/Utils.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,19 @@ bool hasFunctionBody(clang::Decl *D);
104104

105105
std::string getStorageQualifierString(clang::Decl *D);
106106

107-
bool getAbsoluteFilePath(std::string FileName, std::string &AbsoluteFp);
107+
// Use this version for user input that has not yet been validated.
108+
std::error_code tryGetCanonicalFilePath(const std::string &FileName,
109+
std::string &AbsoluteFp);
110+
111+
// This version asserts success. It may be used for convenience for files we are
112+
// already accessing and thus believe should exist, modulo race conditions and
113+
// the like.
114+
void getCanonicalFilePath(const std::string &FileName, std::string &AbsoluteFp);
115+
116+
// This compares entire path components: it's smart enough to know that "foo.c"
117+
// does not start with "foo". It's not smart about anything else, so you should
118+
// probably put both paths through getCanonicalFilePath first.
119+
bool filePathStartsWith(const std::string &Path, const std::string &Prefix);
108120

109121
bool isNULLExpression(clang::Expr *E, clang::ASTContext &C);
110122

@@ -147,6 +159,9 @@ bool isCastSafe(clang::QualType DstType, clang::QualType SrcType);
147159

148160
// Check if the provided file path belongs to the input project
149161
// and can be rewritten.
162+
//
163+
// For accurate results, the path must have gone through getCanonicalFilePath.
164+
// Note that the file name of a PersistentSourceLoc is always canonical.
150165
bool canWrite(const std::string &FilePath);
151166

152167
// Check if the provided variable has void as one of its type.

clang/lib/3C/3C.cpp

Lines changed: 99 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ static cl::opt<bool>
3838
bool DumpIntermediate;
3939
bool Verbose;
4040
std::string OutputPostfix;
41+
std::string OutputDir;
4142
std::string ConstraintOutputJson;
4243
std::vector<std::string> AllocatorFunctions;
4344
bool DumpStats;
@@ -89,7 +90,7 @@ class RewriteAction : public ASTFrontendAction {
8990

9091
virtual std::unique_ptr<ASTConsumer>
9192
CreateASTConsumer(CompilerInstance &Compiler, StringRef InFile) {
92-
return std::unique_ptr<ASTConsumer>(new T(Info, OutputPostfix));
93+
return std::unique_ptr<ASTConsumer>(new T(Info));
9394
}
9495

9596
private:
@@ -194,13 +195,28 @@ void runSolver(ProgramInfo &Info, std::set<std::string> &SourceFiles) {
194195
}
195196
}
196197

198+
_3CInterface *
199+
_3CInterface::create(const struct _3COptions &CCopt,
200+
const std::vector<std::string> &SourceFileList,
201+
CompilationDatabase *CompDB) {
202+
bool Failed = false;
203+
_3CInterface *_3CInter =
204+
new _3CInterface(CCopt, SourceFileList, CompDB, Failed);
205+
if (Failed) {
206+
delete _3CInter;
207+
return nullptr;
208+
}
209+
return _3CInter;
210+
}
211+
197212
_3CInterface::_3CInterface(const struct _3COptions &CCopt,
198213
const std::vector<std::string> &SourceFileList,
199-
CompilationDatabase *CompDB) {
214+
CompilationDatabase *CompDB, bool &Failed) {
200215

201216
DumpIntermediate = CCopt.DumpIntermediate;
202217
Verbose = CCopt.Verbose;
203218
OutputPostfix = CCopt.OutputPostfix;
219+
OutputDir = CCopt.OutputDir;
204220
ConstraintOutputJson = CCopt.ConstraintOutputJson;
205221
StatsOutputJson = CCopt.StatsOutputJson;
206222
WildPtrInfoJson = CCopt.WildPtrInfoJson;
@@ -231,35 +247,97 @@ _3CInterface::_3CInterface(const struct _3COptions &CCopt,
231247

232248
ConstraintsBuilt = false;
233249

234-
// Get the absolute path of the base directory.
235-
std::string TmpPath = BaseDir;
236-
getAbsoluteFilePath(BaseDir, TmpPath);
237-
BaseDir = TmpPath;
250+
if (OutputPostfix != "-" && !OutputDir.empty()) {
251+
errs() << "3C initialization error: Cannot use both -output-postfix and "
252+
"-output-dir\n";
253+
Failed = true;
254+
return;
255+
}
256+
if (OutputPostfix == "-" && OutputDir.empty() && SourceFileList.size() > 1) {
257+
errs() << "3C initialization error: Cannot specify more than one input "
258+
"file when output is to stdout\n";
259+
Failed = true;
260+
return;
261+
}
262+
263+
std::string TmpPath;
264+
std::error_code EC;
238265

239266
if (BaseDir.empty()) {
240-
SmallString<256> Cp;
241-
if (std::error_code Ec = sys::fs::current_path(Cp)) {
242-
errs() << "could not get current working dir\n";
243-
assert(false && "Unable to get determine working directory.");
244-
}
267+
BaseDir = ".";
268+
}
269+
270+
// Get the canonical path of the base directory.
271+
TmpPath = BaseDir;
272+
EC = tryGetCanonicalFilePath(BaseDir, TmpPath);
273+
if (EC) {
274+
errs() << "3C initialization error: Failed to canonicalize base directory "
275+
"\"" << BaseDir << "\": " << EC.message() << "\n";
276+
Failed = true;
277+
return;
278+
}
279+
BaseDir = TmpPath;
245280

246-
BaseDir = Cp.str();
281+
if (!OutputDir.empty()) {
282+
// tryGetCanonicalFilePath will fail if the output dir doesn't exist yet, so
283+
// create it first.
284+
EC = llvm::sys::fs::create_directories(OutputDir);
285+
if (EC) {
286+
errs() << "3C initialization error: Failed to create output directory \""
287+
<< OutputDir << "\": " << EC.message() << "\n";
288+
Failed = true;
289+
return;
290+
}
291+
TmpPath = OutputDir;
292+
EC = tryGetCanonicalFilePath(OutputDir, TmpPath);
293+
if (EC) {
294+
errs() << "3C initialization error: Failed to canonicalize output "
295+
"directory \"" << OutputDir << "\": " << EC.message() << "\n";
296+
Failed = true;
297+
return;
298+
}
299+
OutputDir = TmpPath;
247300
}
248301

249302
SourceFiles = SourceFileList;
250303

304+
bool SawInputOutsideBaseDir = false;
251305
for (const auto &S : SourceFiles) {
252306
std::string AbsPath;
253-
if (getAbsoluteFilePath(S, AbsPath))
254-
FilePaths.insert(AbsPath);
307+
EC = tryGetCanonicalFilePath(S, AbsPath);
308+
if (EC) {
309+
errs() << "3C initialization error: Failed to canonicalize source file "
310+
"path \"" << S << "\": " << EC.message() << "\n";
311+
Failed = true;
312+
continue;
313+
}
314+
FilePaths.insert(AbsPath);
315+
if (!filePathStartsWith(AbsPath, BaseDir)) {
316+
errs()
317+
<< "3C initialization "
318+
<< (OutputDir != "" || !CCopt.AllowSourcesOutsideBaseDir ? "error"
319+
: "warning")
320+
<< ": File \"" << AbsPath
321+
<< "\" specified on the command line is outside the base directory\n";
322+
SawInputOutsideBaseDir = true;
323+
}
324+
}
325+
if (SawInputOutsideBaseDir) {
326+
errs() << "The base directory is currently \"" << BaseDir
327+
<< "\" and can be changed with the -base-dir option.\n";
328+
if (OutputDir != "") {
329+
Failed = true;
330+
errs() << "When using -output-dir, input files outside the base "
331+
"directory cannot be handled because there is no way to "
332+
"compute their output paths.\n";
333+
} else if (!CCopt.AllowSourcesOutsideBaseDir) {
334+
Failed = true;
335+
errs() << "You can use the -allow-sources-outside-base-dir option to "
336+
"temporarily downgrade this error to a warning.\n";
337+
}
255338
}
256339

257340
CurrCompDB = CompDB;
258-
259-
if (OutputPostfix == "-" && FilePaths.size() > 1) {
260-
errs() << "If rewriting more than one , can't output to stdout\n";
261-
assert(false && "Rewriting more than one files requires OutputPostfix");
262-
}
263341
}
264342

265343
bool _3CInterface::buildInitialConstraints() {
@@ -295,15 +373,15 @@ bool _3CInterface::solveConstraints() {
295373
"build constraint before trying to solve them.");
296374
// 2. Solve constraints.
297375
if (Verbose)
298-
outs() << "Solving constraints\n";
376+
errs() << "Solving constraints\n";
299377

300378
if (DumpIntermediate)
301379
GlobalProgramInfo.dump();
302380

303381
runSolver(GlobalProgramInfo, FilePaths);
304382

305383
if (Verbose)
306-
outs() << "Constraints solved\n";
384+
errs() << "Constraints solved\n";
307385

308386
if (WarnRootCause)
309387
GlobalProgramInfo.computeInterimConstraintState(FilePaths);

clang/lib/3C/ConstraintBuilder.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,7 @@ void ConstraintBuilderConsumer::HandleTranslationUnit(ASTContext &C) {
660660
TV.setProgramInfoTypeVars();
661661

662662
if (Verbose)
663-
outs() << "Done analyzing\n";
663+
errs() << "Done analyzing\n";
664664

665665
Info.exitCompilationUnit();
666666
return;

clang/lib/3C/PersistentSourceLoc.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ PersistentSourceLoc PersistentSourceLoc::mkPSL(clang::SourceRange SR,
7070
std::string FeAbsS = "";
7171
if (Fe != nullptr)
7272
ToConv = Fe->getName();
73-
if (getAbsoluteFilePath(ToConv, FeAbsS))
74-
Fn = sys::path::remove_leading_dotslash(FeAbsS);
73+
getCanonicalFilePath(ToConv, FeAbsS);
74+
Fn = sys::path::remove_leading_dotslash(FeAbsS);
7575
}
7676
PersistentSourceLoc PSL(Fn, FESL.getExpansionLineNumber(),
7777
FESL.getExpansionColumnNumber(), EndCol);

0 commit comments

Comments
 (0)